| Message ID | 20221109190849.1556711-1-conor@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/misc/pfsoc: add fabric clocks to ioscb | expand |
Hi Conor, On 9/11/22 20:08, Conor Dooley wrote: > From: Conor Dooley <conor.dooley@microchip.com> > > On PolarFire SoC, some peripherals (eg the PCI root port) are clocked by > "Clock Conditioning Circuitry" in the FPGA. The specific clock depends > on the FPGA bitstream & can be locked to one particular {D,P}LL - in the > Icicle Kit Reference Design v2022.09 or later this is/will be the case. > > Linux v6.1+ will have a driver for this peripheral and devicetrees that > previously relied on "fixed-frequency" clock nodes have been switched > over to clock-controller nodes. The IOSCB region is represented in QEMU, > but the specific region of it that the CCCs occupy has not so v6.1-rcN > kernels fail to boot in QEMU. > > Add the regions as unimplemented so that the status-quo in terms of boot > is maintained. > > Signed-off-by: Conor Dooley <conor.dooley@microchip.com> > --- > The last line there is a white lie. v6.1-rcN has both v2022.09 and > v2022.10 reference design changes. This patch only accounts for the > v2022.09 changes. The FPGA design is a moving target and I am not > really sure how to handle that in QEMU. For v2022.10 a bunch of stuff > got changed, including the addresses that DDR lies at which I am not > sure how to handle yet. > > That puts my todo list of broken things to: > - MMC (only direct kernel boot works), pre v2022.09 reference issue How do you start without 'direct kernel boot'? > - PCI root port address, address changed in v2022.09 but from a cursory > check, I didn't see any PCI support in the first place. It's connected > to a FIC, so I think it can just be made into an unimplemented region. > - DDR address changes, 2022.10 issue. Looks like a straightforward > change to hw/riscv/pfsoc.c but I don't think it'll be backwards > compatible. > - hwrng breaks boot. Tipping away at this one, hopefully I'll have a fix > for it soon. Need to implement the irq side of the mailbox for it. > > I'll send some more patches as I work through them. > > hw/misc/mchp_pfsoc_ioscb.c | 6 ++++++ > include/hw/misc/mchp_pfsoc_ioscb.h | 1 + > 2 files changed, 7 insertions(+) > > diff --git a/hw/misc/mchp_pfsoc_ioscb.c b/hw/misc/mchp_pfsoc_ioscb.c > index f4fd55a0e5..f976e42f72 100644 > --- a/hw/misc/mchp_pfsoc_ioscb.c > +++ b/hw/misc/mchp_pfsoc_ioscb.c > @@ -33,6 +33,7 @@ > */ > #define IOSCB_WHOLE_REG_SIZE 0x10000000 > #define IOSCB_SUBMOD_REG_SIZE 0x1000 > +#define IOSCB_CCC_REG_SIZE 0x2000000 > > /* > * There are many sub-modules in the IOSCB module. > @@ -45,6 +46,7 @@ > #define IOSCB_LANE23_BASE 0x06510000 > #define IOSCB_CTRL_BASE 0x07020000 > #define IOSCB_CFG_BASE 0x07080000 > +#define IOSCB_CCC_BASE 0x08000000 > #define IOSCB_PLL_MSS_BASE 0x0E001000 > #define IOSCB_CFM_MSS_BASE 0x0E002000 > #define IOSCB_PLL_DDR_BASE 0x0E010000 > @@ -168,6 +170,10 @@ static void mchp_pfsoc_ioscb_realize(DeviceState *dev, Error **errp) > "mchp.pfsoc.ioscb.cfg", IOSCB_SUBMOD_REG_SIZE); > memory_region_add_subregion(&s->container, IOSCB_CFG_BASE, &s->cfg); > > + memory_region_init_io(&s->ccc, OBJECT(s), &mchp_pfsoc_dummy_ops, s, > + "mchp.pfsoc.ioscb.ccc", IOSCB_CCC_REG_SIZE); > + memory_region_add_subregion(&s->container, IOSCB_CCC_BASE, &s->ccc); Unrelated but using the TYPE_UNIMPLEMENTED_DEVICE would ease tracing all these block accesses, as the block name would appear before the address/size. See for example aspeed_mmio_map_unimplemented(); Otherwise LGTM. Regards, Phil. > memory_region_init_io(&s->pll_mss, OBJECT(s), &mchp_pfsoc_pll_ops, s, > "mchp.pfsoc.ioscb.pll_mss", IOSCB_SUBMOD_REG_SIZE); > memory_region_add_subregion(&s->container, IOSCB_PLL_MSS_BASE, &s->pll_mss); > diff --git a/include/hw/misc/mchp_pfsoc_ioscb.h b/include/hw/misc/mchp_pfsoc_ioscb.h > index 9235523e33..687b213742 100644 > --- a/include/hw/misc/mchp_pfsoc_ioscb.h > +++ b/include/hw/misc/mchp_pfsoc_ioscb.h > @@ -30,6 +30,7 @@ typedef struct MchpPfSoCIoscbState { > MemoryRegion lane23; > MemoryRegion ctrl; > MemoryRegion cfg; > + MemoryRegion ccc; > MemoryRegion pll_mss; > MemoryRegion cfm_mss; > MemoryRegion pll_ddr;
On Thu, Nov 10, 2022 at 12:18:44AM +0100, Philippe Mathieu-Daudé wrote: > On 9/11/22 20:08, Conor Dooley wrote: > > From: Conor Dooley <conor.dooley@microchip.com> > > > > On PolarFire SoC, some peripherals (eg the PCI root port) are clocked by > > "Clock Conditioning Circuitry" in the FPGA. The specific clock depends > > on the FPGA bitstream & can be locked to one particular {D,P}LL - in the > > Icicle Kit Reference Design v2022.09 or later this is/will be the case. > > > > Linux v6.1+ will have a driver for this peripheral and devicetrees that > > previously relied on "fixed-frequency" clock nodes have been switched > > over to clock-controller nodes. The IOSCB region is represented in QEMU, > > but the specific region of it that the CCCs occupy has not so v6.1-rcN > > kernels fail to boot in QEMU. > > > > Add the regions as unimplemented so that the status-quo in terms of boot > > is maintained. > > > > Signed-off-by: Conor Dooley <conor.dooley@microchip.com> > > --- > > The last line there is a white lie. v6.1-rcN has both v2022.09 and > > v2022.10 reference design changes. This patch only accounts for the > > v2022.09 changes. The FPGA design is a moving target and I am not > > really sure how to handle that in QEMU. For v2022.10 a bunch of stuff > > got changed, including the addresses that DDR lies at which I am not > > sure how to handle yet. > > > > That puts my todo list of broken things to: > > - MMC (only direct kernel boot works), pre v2022.09 reference issue > > How do you start without 'direct kernel boot'? You used to be able to load the "bios" etc and follow the boot flow [0]. This no longer works, and has not for at least a year. I assume it still works if you check out the (fossilised) versions mentioned in that doc. Think I said it last time I sent patches, but we had some floating around internally that I know /did/ work at some point about this time last year but I was never able to figure out the correct alignment of the stars to get working myself. It required pretty decent changes to the sdhci driver, which, I'm hoping cease to be required with the v2022.10 reference design that I mentioned else where in this patch. But one problem at a time ;) 0 - https://www.qemu.org/docs/master/system/riscv/microchip-icicle-kit.html > > - PCI root port address, address changed in v2022.09 but from a cursory > > check, I didn't see any PCI support in the first place. It's connected > > to a FIC, so I think it can just be made into an unimplemented region. > > - DDR address changes, 2022.10 issue. Looks like a straightforward > > change to hw/riscv/pfsoc.c but I don't think it'll be backwards > > compatible. > > - hwrng breaks boot. Tipping away at this one, hopefully I'll have a fix > > for it soon. Need to implement the irq side of the mailbox for it. > > > > I'll send some more patches as I work through them. > > > > hw/misc/mchp_pfsoc_ioscb.c | 6 ++++++ > > include/hw/misc/mchp_pfsoc_ioscb.h | 1 + > > 2 files changed, 7 insertions(+) > > > > diff --git a/hw/misc/mchp_pfsoc_ioscb.c b/hw/misc/mchp_pfsoc_ioscb.c > > index f4fd55a0e5..f976e42f72 100644 > > --- a/hw/misc/mchp_pfsoc_ioscb.c > > +++ b/hw/misc/mchp_pfsoc_ioscb.c > > @@ -33,6 +33,7 @@ > > */ > > #define IOSCB_WHOLE_REG_SIZE 0x10000000 > > #define IOSCB_SUBMOD_REG_SIZE 0x1000 > > +#define IOSCB_CCC_REG_SIZE 0x2000000 > > /* > > * There are many sub-modules in the IOSCB module. > > @@ -45,6 +46,7 @@ > > #define IOSCB_LANE23_BASE 0x06510000 > > #define IOSCB_CTRL_BASE 0x07020000 > > #define IOSCB_CFG_BASE 0x07080000 > > +#define IOSCB_CCC_BASE 0x08000000 > > #define IOSCB_PLL_MSS_BASE 0x0E001000 > > #define IOSCB_CFM_MSS_BASE 0x0E002000 > > #define IOSCB_PLL_DDR_BASE 0x0E010000 > > @@ -168,6 +170,10 @@ static void mchp_pfsoc_ioscb_realize(DeviceState *dev, Error **errp) > > "mchp.pfsoc.ioscb.cfg", IOSCB_SUBMOD_REG_SIZE); > > memory_region_add_subregion(&s->container, IOSCB_CFG_BASE, &s->cfg); > > + memory_region_init_io(&s->ccc, OBJECT(s), &mchp_pfsoc_dummy_ops, s, > > + "mchp.pfsoc.ioscb.ccc", IOSCB_CCC_REG_SIZE); > > + memory_region_add_subregion(&s->container, IOSCB_CCC_BASE, &s->ccc); > > Unrelated but using the TYPE_UNIMPLEMENTED_DEVICE would ease tracing all > these block accesses, as the block name would appear before the > address/size. See for example aspeed_mmio_map_unimplemented(); I just copy pasted what was already here. Follow on patch for the conversions since most of what's in this file is effectively unimplemented? > > memory_region_init_io(&s->pll_mss, OBJECT(s), &mchp_pfsoc_pll_ops, s, > > "mchp.pfsoc.ioscb.pll_mss", IOSCB_SUBMOD_REG_SIZE); > > memory_region_add_subregion(&s->container, IOSCB_PLL_MSS_BASE, &s->pll_mss); > > diff --git a/include/hw/misc/mchp_pfsoc_ioscb.h b/include/hw/misc/mchp_pfsoc_ioscb.h > > index 9235523e33..687b213742 100644 > > --- a/include/hw/misc/mchp_pfsoc_ioscb.h > > +++ b/include/hw/misc/mchp_pfsoc_ioscb.h > > @@ -30,6 +30,7 @@ typedef struct MchpPfSoCIoscbState { > > MemoryRegion lane23; > > MemoryRegion ctrl; > > MemoryRegion cfg; > > + MemoryRegion ccc; > > MemoryRegion pll_mss; > > MemoryRegion cfm_mss; > > MemoryRegion pll_ddr; >
On Thu, Nov 10, 2022 at 12:18:44AM +0100, Philippe Mathieu-Daudé wrote: > Hi Conor, > > On 9/11/22 20:08, Conor Dooley wrote: > > From: Conor Dooley <conor.dooley@microchip.com> > > > > @@ -168,6 +170,10 @@ static void mchp_pfsoc_ioscb_realize(DeviceState *dev, Error **errp) > > "mchp.pfsoc.ioscb.cfg", IOSCB_SUBMOD_REG_SIZE); > > memory_region_add_subregion(&s->container, IOSCB_CFG_BASE, &s->cfg); > > + memory_region_init_io(&s->ccc, OBJECT(s), &mchp_pfsoc_dummy_ops, s, > > + "mchp.pfsoc.ioscb.ccc", IOSCB_CCC_REG_SIZE); > > + memory_region_add_subregion(&s->container, IOSCB_CCC_BASE, &s->ccc); > > Unrelated but using the TYPE_UNIMPLEMENTED_DEVICE would ease tracing all > these block accesses, as the block name would appear before the > address/size. See for example aspeed_mmio_map_unimplemented(); Certainly looks like a nice idea, and I gave it a go but kept running into issues due to my lack of understanding of QEMU :) I'm going to add this to my todo pile - while I have a v2 of this lined up, I'd rather not hold up adding the regions that prevent booting Linux etc as I fumble around trying to understand the hierarchy of devices required to set up something similar to your aspeed example. Thanks, Conor.
Hi Conor, On Sat, Nov 12, 2022 at 8:31 AM Conor Dooley <conor@kernel.org> wrote: > > On Thu, Nov 10, 2022 at 12:18:44AM +0100, Philippe Mathieu-Daudé wrote: > > Hi Conor, > > > > On 9/11/22 20:08, Conor Dooley wrote: > > > From: Conor Dooley <conor.dooley@microchip.com> > > > > > > @@ -168,6 +170,10 @@ static void mchp_pfsoc_ioscb_realize(DeviceState *dev, Error **errp) > > > "mchp.pfsoc.ioscb.cfg", IOSCB_SUBMOD_REG_SIZE); > > > memory_region_add_subregion(&s->container, IOSCB_CFG_BASE, &s->cfg); > > > + memory_region_init_io(&s->ccc, OBJECT(s), &mchp_pfsoc_dummy_ops, s, > > > + "mchp.pfsoc.ioscb.ccc", IOSCB_CCC_REG_SIZE); > > > + memory_region_add_subregion(&s->container, IOSCB_CCC_BASE, &s->ccc); > > > > Unrelated but using the TYPE_UNIMPLEMENTED_DEVICE would ease tracing all > > these block accesses, as the block name would appear before the > > address/size. See for example aspeed_mmio_map_unimplemented(); > > Certainly looks like a nice idea, and I gave it a go but kept running > into issues due to my lack of understanding of QEMU :) I'm going to add > this to my todo pile - while I have a v2 of this lined up, I'd rather > not hold up adding the regions that prevent booting Linux etc as I > fumble around trying to understand the hierarchy of devices required to > set up something similar to your aspeed example. > Do you plan to bring QEMU support to the latest MSS_LINUX configuration [1] Currently QEMU is supporting the MSS_BAREMETAL configuration. Do you think it makes sense to support both? [1] https://github.com/polarfire-soc/icicle-kit-reference-design Regards, Bin
On Sat, Nov 12, 2022 at 08:37:38AM +0800, Bin Meng wrote: > Hi Conor, > > On Sat, Nov 12, 2022 at 8:31 AM Conor Dooley <conor@kernel.org> wrote: > > > > On Thu, Nov 10, 2022 at 12:18:44AM +0100, Philippe Mathieu-Daudé wrote: > > > Hi Conor, > > > > > > On 9/11/22 20:08, Conor Dooley wrote: > > > > From: Conor Dooley <conor.dooley@microchip.com> > > > > > > > > @@ -168,6 +170,10 @@ static void mchp_pfsoc_ioscb_realize(DeviceState *dev, Error **errp) > > > > "mchp.pfsoc.ioscb.cfg", IOSCB_SUBMOD_REG_SIZE); > > > > memory_region_add_subregion(&s->container, IOSCB_CFG_BASE, &s->cfg); > > > > + memory_region_init_io(&s->ccc, OBJECT(s), &mchp_pfsoc_dummy_ops, s, > > > > + "mchp.pfsoc.ioscb.ccc", IOSCB_CCC_REG_SIZE); > > > > + memory_region_add_subregion(&s->container, IOSCB_CCC_BASE, &s->ccc); > > > > > > Unrelated but using the TYPE_UNIMPLEMENTED_DEVICE would ease tracing all > > > these block accesses, as the block name would appear before the > > > address/size. See for example aspeed_mmio_map_unimplemented(); > > > > Certainly looks like a nice idea, and I gave it a go but kept running > > into issues due to my lack of understanding of QEMU :) I'm going to add > > this to my todo pile - while I have a v2 of this lined up, I'd rather > > not hold up adding the regions that prevent booting Linux etc as I > > fumble around trying to understand the hierarchy of devices required to > > set up something similar to your aspeed example. > > > > Do you plan to bring QEMU support to the latest MSS_LINUX configuration [1] "Yes". Our goal is to merge both the LINUX and BAREMETAL configurations in an upcoming reference design release. Notably absent from anything that I have sent here is any changes to the DDR configuration (and that and how the PCI root port is connected to the MSS are the only real differences between the two). Currently, the LINUX one has 2 GiB of DDR at 0x10_0000_0000 & that's what the vendor kernel uses. None of upstream Linux, U-Boot or QEMU support that configuration. The baremetal one has 1 GiB at 0x8000_0000 and 1 GiB at 0x10_0000_0000. When the two are merged, it'll be 1 GiB at 0x8000_0000 and 1 GiB at 0x10_4000_0000 - there's currently a v2022.10 reference design job file on [1] that's got this configuration but we are waiting for a corresponding release of Libero to properly release the tcl scripts etc. We're upstreaming U-Boot and Linux support for that configuration at the moment - but it's just a dts change there so no real concern about breaking any backwards compat as the older devicetrees will continue to work. > Currently QEMU is supporting the MSS_BAREMETAL configuration. Do you > think it makes sense to support both? > [1] https://github.com/polarfire-soc/icicle-kit-reference-design I was kinda hoping to leave that part of things as-is for now and wait for the merged configuration. My main question with that is: do the older reference design configurations need to remain supported? The PCI root port stuff likely doesn't matter since it's not modelled (yet) by QEMU anyway but the DDR bit is going to be incompatible. The addresses at which DDR lies are controlled by the seg registers. These are briefly documented in the TRM (4.5 Segmentation Blocks) but IMO pretty badly explained there. IIUC, for bare metal applications that's set by the HAL from the XML exported by MSS configurator & for anything started via the HSS, the HSS does it instead. I was thinking something like defaulting the DDR configuration to the new, merged configuration & then if someone writes to the seg registers (which, IIUC, a bare-metal app does) changing the addresses at which QEMU places the DDR at runtime. That's what the hardware does, but I have put approximately zero thought into how to implement that. Without something like that, idk how we'd keep both newer and older reference designs working in QEMU. > Do you plan to bring QEMU support to the latest MSS_LINUX configuration Either way, any plans are dependant on me finding the time. I'm mostly just upstreaming the small changes that I need to make so that QEMU remains usable as a debugging tool for Linux stuff. Thanks, Conor.
diff --git a/hw/misc/mchp_pfsoc_ioscb.c b/hw/misc/mchp_pfsoc_ioscb.c index f4fd55a0e5..f976e42f72 100644 --- a/hw/misc/mchp_pfsoc_ioscb.c +++ b/hw/misc/mchp_pfsoc_ioscb.c @@ -33,6 +33,7 @@ */ #define IOSCB_WHOLE_REG_SIZE 0x10000000 #define IOSCB_SUBMOD_REG_SIZE 0x1000 +#define IOSCB_CCC_REG_SIZE 0x2000000 /* * There are many sub-modules in the IOSCB module. @@ -45,6 +46,7 @@ #define IOSCB_LANE23_BASE 0x06510000 #define IOSCB_CTRL_BASE 0x07020000 #define IOSCB_CFG_BASE 0x07080000 +#define IOSCB_CCC_BASE 0x08000000 #define IOSCB_PLL_MSS_BASE 0x0E001000 #define IOSCB_CFM_MSS_BASE 0x0E002000 #define IOSCB_PLL_DDR_BASE 0x0E010000 @@ -168,6 +170,10 @@ static void mchp_pfsoc_ioscb_realize(DeviceState *dev, Error **errp) "mchp.pfsoc.ioscb.cfg", IOSCB_SUBMOD_REG_SIZE); memory_region_add_subregion(&s->container, IOSCB_CFG_BASE, &s->cfg); + memory_region_init_io(&s->ccc, OBJECT(s), &mchp_pfsoc_dummy_ops, s, + "mchp.pfsoc.ioscb.ccc", IOSCB_CCC_REG_SIZE); + memory_region_add_subregion(&s->container, IOSCB_CCC_BASE, &s->ccc); + memory_region_init_io(&s->pll_mss, OBJECT(s), &mchp_pfsoc_pll_ops, s, "mchp.pfsoc.ioscb.pll_mss", IOSCB_SUBMOD_REG_SIZE); memory_region_add_subregion(&s->container, IOSCB_PLL_MSS_BASE, &s->pll_mss); diff --git a/include/hw/misc/mchp_pfsoc_ioscb.h b/include/hw/misc/mchp_pfsoc_ioscb.h index 9235523e33..687b213742 100644 --- a/include/hw/misc/mchp_pfsoc_ioscb.h +++ b/include/hw/misc/mchp_pfsoc_ioscb.h @@ -30,6 +30,7 @@ typedef struct MchpPfSoCIoscbState { MemoryRegion lane23; MemoryRegion ctrl; MemoryRegion cfg; + MemoryRegion ccc; MemoryRegion pll_mss; MemoryRegion cfm_mss; MemoryRegion pll_ddr;