[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH v2] pnv: add a physical mapping array describing M
From: |
David Gibson |
Subject: |
Re: [Qemu-ppc] [PATCH v2] pnv: add a physical mapping array describing MMIO ranges in each chip |
Date: |
Thu, 14 Jun 2018 16:44:59 +1000 |
User-agent: |
Mutt/1.10.0 (2018-05-17) |
On Wed, Jun 13, 2018 at 09:03:22AM +0200, Cédric Le Goater wrote:
> On 06/13/2018 02:47 AM, David Gibson wrote:
> > On Tue, Jun 12, 2018 at 08:13:49AM +0200, Cédric Le Goater wrote:
> >> On 06/12/2018 07:58 AM, David Gibson wrote:
> >>> On Wed, Jun 06, 2018 at 09:04:10AM +0200, Cédric Le Goater wrote:
> >>>> On 06/06/2018 08:39 AM, David Gibson wrote:
> >>>>> On Wed, May 30, 2018 at 12:07:54PM +0200, Cédric Le Goater wrote:
> >>>>>> Based on previous work done in skiboot, the physical mapping array
> >>>>>> helps in calculating the MMIO ranges of each controller depending on
> >>>>>> the chip id and the chip type. This is will be particularly useful for
> >>>>>> the P9 models which use less the XSCOM bus and rely more on MMIOs.
> >>>>>>
> >>>>>> A link on the chip is now necessary to calculate MMIO BARs and
> >>>>>> sizes. This is why such a link is introduced in the PSIHB model.
> >>>>>
> >>>>> I think this message needs some work. This says what it's for, but
> >>>>> what actually *is* this array, and how does it work?
> >>>>
> >>>> OK. It is relatively simple: each controller has an entry defining its
> >>>> MMIO range.
> >>>>
> >>>>> The outside-core differences between POWER8 and POWER9 are substantial
> >>>>> enough that I'm wondering if pnvP8 and pnvP9 would be better off as
> >>>>> different machine types (sharing some routines, of course).
> >>>>
> >>>> yes and no. I have survived using a common PnvChip framework but
> >>>> it is true that I had to add P9 classes for each: LPC, PSI, OCC
> >>>> They are very similar but not enough. P9 uses much more MMIOs than
> >>>> P8 which still uses a lot of XSCOM. I haven't looked at PHB4.
> >>>
> >>> Well, it's certainly *possible* to use the same machine type, I'm just
> >>> not convinced it's a good idea. It seems kind of dodgy to me that so
> >>> many peripherals on the system change as a side-effect of setting the
> >>> cpu. Compare to how x86 works where cpu really does change the CPU,
> >>> plugging it into the same virtual "chipset". Different chipsets *are*
> >>> different machine types there (pc vs. q35).
> >>
> >> OK, I agree, and we can use a set of common routines to instantiate the
> >> different chipset models.
> >>
> >> So we would have a common pnv_init() routine to initialize the different
> >> 'powernv8' and 'powernv9' machines and the PnvChip typename would be a
> >> machine class attribute ?
> >
> > Well.. that's one option. Usually for these things, it works out
> > better to instead of parameterizing big high-level routines like
> > pnv_init(), you have separate versions of those calling a combination
> > of case-specific and common routines as necessary.
> >
> > Mostly it just comes down to what is simplest to implement for you, though.
>
> I am introducing a powernv8 machine, the machine init routine is still
> generic and did not change much. But I have deepen the PnvChip class
> inheritance hierarchy with an intermediate class taking care of the
> Chip sub controllers, which gives us something like :
>
> pnv_init()
> . skiboot
> . kernel
> . initrd
> . chips creation
> . platform bus/device :
> isa bus
> pci layout
> bmc handling.
>
> p8 chip hierarchy:
>
> power8_v2.0-pnv-chip (gives the cpu type)
> pnv8-chip : creates the devices only
> pnv-chip : creates xscom and the cores
>
> The powervn9 machine has this hierarchy :
>
> power9_v2.0-pnv-chip
> pnv9-chip
> pnv-chip
>
> I had to introduce these new PnvChipClass ops :
>
> void (*realize)(PnvChip *chip, Error **errp);
Looks sensible up to this point.
> Object *(*intc_create)(PnvChip *chip, Object *child, Error **errp);
> ISABus *(*isa_create)(PnvChip *chip);
I'm a little more dubious about this - I would have thought your
realize() hook could construct the right intc and isa, rather than
going into generic code, then back out to the callback.
But it's not a big deal.
> Overall, it's looking fine and it should remove most of these tests :
>
> pnv_chip_is_power9(chip)
>
> If not, it means we are missing a PnvChipClass ops anyway.
Nice.
> I will send a patchset this week, the final one will shuffle quite a
> bit of code and the resulting diff will be a bit fuzy. You will have
> to trust me on this one.
>
>
> >> Nevertheless we would still need to introduce "a physical mapping array
> >> describing MMIO ranges" but we can start by differentiating the chipsets
> >> first.
> >
> > Well, maybe. I'm wondering if you can more easily encapsulate the
> > information in that array in a top-level init routine, that calls
> > common helpers with different addresses / device types / whatever.
>
> Hmm, I think I understand but could you give me a prototype example.
> Please. To make sure.
>
> I would like to keep the array somewhere because, in a quick look,
> it gives you an overview of the POWER Processor address space.
Ok. Going to a data-driven approach for constructing things sounds
like it might be a reasonable plan in its own right. But I'd prefer
not to conflate with other structural questions.
--
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
- Re: [Qemu-ppc] [PATCH v2] pnv: add a physical mapping array describing MMIO ranges in each chip, Cédric Le Goater, 2018/06/06
- Re: [Qemu-ppc] [PATCH v2] pnv: add a physical mapping array describing MMIO ranges in each chip, David Gibson, 2018/06/06
- Re: [Qemu-ppc] [PATCH v2] pnv: add a physical mapping array describing MMIO ranges in each chip, Cédric Le Goater, 2018/06/06
- Re: [Qemu-ppc] [PATCH v2] pnv: add a physical mapping array describing MMIO ranges in each chip, David Gibson, 2018/06/12
- Re: [Qemu-ppc] [PATCH v2] pnv: add a physical mapping array describing MMIO ranges in each chip, Cédric Le Goater, 2018/06/12
- Re: [Qemu-ppc] [PATCH v2] pnv: add a physical mapping array describing MMIO ranges in each chip, David Gibson, 2018/06/12
- Re: [Qemu-ppc] [PATCH v2] pnv: add a physical mapping array describing MMIO ranges in each chip, Cédric Le Goater, 2018/06/13
- Re: [Qemu-ppc] [PATCH v2] pnv: add a physical mapping array describing MMIO ranges in each chip,
David Gibson <=
- Re: [Qemu-ppc] [PATCH v2] pnv: add a physical mapping array describing MMIO ranges in each chip, Cédric Le Goater, 2018/06/14