[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH v3 1/3] spapr: introduce a fixed IRQ number space
From: |
David Gibson |
Subject: |
Re: [Qemu-ppc] [PATCH v3 1/3] spapr: introduce a fixed IRQ number space |
Date: |
Fri, 6 Jul 2018 15:44:58 +1000 |
User-agent: |
Mutt/1.10.0 (2018-05-17) |
On Tue, Jul 03, 2018 at 05:19:56PM +0200, Cédric Le Goater wrote:
> On 07/02/2018 01:11 PM, Cédric Le Goater wrote:
> > On 07/02/2018 12:03 PM, Cédric Le Goater wrote:
> >>> --- a/hw/ppc/spapr_vio.c
> >>> +++ b/hw/ppc/spapr_vio.c
> >>> @@ -436,6 +436,9 @@ static void spapr_vio_busdev_reset(DeviceState *qdev)
> >>> }
> >>> }
> >>>
> >>> +/* TODO : poor VIO device indexing ... */
> >>> +static uint32_t vio_index;
> >>
> >> I think we could also use (dev->reg & 0xff) as an index for
> >> the VIO devices.
> >>
> >> The unit address of the virtual IOA is simply allocated using
> >> an increment of bus->next_reg, next_reg being initialized at
> >> 0x71000000.
> >>
> >> I did not see any restrictions in the PAPR specs or in QEMU
> >> that would break the above.
> >
> > That was until I discovered this macro :
> >
> > #define DEFINE_SPAPR_PROPERTIES(type, field) \
> > DEFINE_PROP_UINT32("reg", type, field.reg, -1)
> >
> > so 'reg' could have any value. We can not use it ...
>
> Would moving vio_index under the bus and incrementing it each time
> a VIO device is created be acceptable ?
Not really, no.
> It does look like an allocator but I really don't know what else to
> propose :/ See below.
Not only is it a stealth allocator, it also means we have two
different unique ids for VIO devices - the 'reg' and this new index.
That sounds like a recipe for confusion.
I think we can do better. I had a look at how these are allocated and
it seems to be this:
In qemu:
VIO devices start at reg=0x71000000, and just increment by one
from there.
In libvirt:
VIO net devices start at reg=0x1000
VIO scsi devices start at reg=0x2000
VIO nvram devices start at reg=0x3000
VIO vty devices start at reg=0x30000000
and increment by 0x1000 each type
So we could go for say:
irq = (reg & 0xf) ^ ((reg >> 12) & 0xf);
Obviously it's easily to construct cases where that will result in
collisions, but I don't think it'll happen for anyone not going out of
there way to make it happen.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature