diff mbox series

[v1] Add dummy Aspeed AST2600 Display Port MCU (DPMCU)

Message ID 20211210083034.726610-1-troy_lee@aspeedtech.com (mailing list archive)
State New, archived
Headers show
Series [v1] Add dummy Aspeed AST2600 Display Port MCU (DPMCU) | expand

Commit Message

Troy Lee Dec. 10, 2021, 8:30 a.m. UTC
AST2600 Display Port MCU introduces 0x18000000~0x1803FFFF as it's memory
and io address. If guest machine try to access DPMCU memory, it will
cause a fatal error.

Signed-off-by: Troy Lee <troy_lee@aspeedtech.com>
---
 hw/arm/aspeed_ast2600.c     | 8 ++++++++
 include/hw/arm/aspeed_soc.h | 2 ++
 2 files changed, 10 insertions(+)

Comments

Philippe Mathieu-Daudé Dec. 10, 2021, 12:58 p.m. UTC | #1
On 12/10/21 09:30, Troy Lee wrote:
> AST2600 Display Port MCU introduces 0x18000000~0x1803FFFF as it's memory
> and io address. If guest machine try to access DPMCU memory, it will
> cause a fatal error.
> 
> Signed-off-by: Troy Lee <troy_lee@aspeedtech.com>
> ---
>  hw/arm/aspeed_ast2600.c     | 8 ++++++++
>  include/hw/arm/aspeed_soc.h | 2 ++
>  2 files changed, 10 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Cédric Le Goater Dec. 10, 2021, 2:05 p.m. UTC | #2
On 12/10/21 09:30, Troy Lee wrote:
> AST2600 Display Port MCU introduces 0x18000000~0x1803FFFF as it's memory
> and io address. If guest machine try to access DPMCU memory, it will
> cause a fatal error.

The Aspeed SoCs have an "aspeed_soc.io" region for unimplemented devices
but it's too small. Anyhow, it is better to have per logic unit. We should
change that one day.

For my information, which FW image are you using ?

> Signed-off-by: Troy Lee <troy_lee@aspeedtech.com>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>   hw/arm/aspeed_ast2600.c     | 8 ++++++++
>   include/hw/arm/aspeed_soc.h | 2 ++
>   2 files changed, 10 insertions(+)
> 
> diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> index 0384357a95..e33483fb5d 100644
> --- a/hw/arm/aspeed_ast2600.c
> +++ b/hw/arm/aspeed_ast2600.c
> @@ -19,9 +19,11 @@
>   #include "sysemu/sysemu.h"
>   
>   #define ASPEED_SOC_IOMEM_SIZE       0x00200000
> +#define ASPEED_SOC_DPMCU_SIZE       0x00040000
>   
>   static const hwaddr aspeed_soc_ast2600_memmap[] = {
>       [ASPEED_DEV_SRAM]      = 0x10000000,
> +    [ASPEED_DEV_DPMCU]     = 0x18000000,
>       /* 0x16000000     0x17FFFFFF : AHB BUS do LPC Bus bridge */
>       [ASPEED_DEV_IOMEM]     = 0x1E600000,
>       [ASPEED_DEV_PWM]       = 0x1E610000,
> @@ -44,6 +46,7 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = {
>       [ASPEED_DEV_SCU]       = 0x1E6E2000,
>       [ASPEED_DEV_XDMA]      = 0x1E6E7000,
>       [ASPEED_DEV_ADC]       = 0x1E6E9000,
> +    [ASPEED_DEV_DP]        = 0x1E6EB000,
>       [ASPEED_DEV_VIDEO]     = 0x1E700000,
>       [ASPEED_DEV_SDHCI]     = 0x1E740000,
>       [ASPEED_DEV_EMMC]      = 0x1E750000,
> @@ -104,6 +107,7 @@ static const int aspeed_soc_ast2600_irqmap[] = {
>       [ASPEED_DEV_ETH3]      = 32,
>       [ASPEED_DEV_ETH4]      = 33,
>       [ASPEED_DEV_KCS]       = 138,   /* 138 -> 142 */
> +    [ASPEED_DEV_DP]        = 62,
>   };
>   
>   static qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int ctrl)
> @@ -298,6 +302,10 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>       memory_region_add_subregion(get_system_memory(),
>                                   sc->memmap[ASPEED_DEV_SRAM], &s->sram);
>   
> +    /* DPMCU */
> +    create_unimplemented_device("aspeed.dpmcu", sc->memmap[ASPEED_DEV_DPMCU],
> +                                ASPEED_SOC_DPMCU_SIZE);
> +
>       /* SCU */
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->scu), errp)) {
>           return;
> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> index 8139358549..18fb7eed46 100644
> --- a/include/hw/arm/aspeed_soc.h
> +++ b/include/hw/arm/aspeed_soc.h
> @@ -139,6 +139,8 @@ enum {
>       ASPEED_DEV_EMMC,
>       ASPEED_DEV_KCS,
>       ASPEED_DEV_HACE,
> +    ASPEED_DEV_DPMCU,
> +    ASPEED_DEV_DP,
>   };
>   
>   #endif /* ASPEED_SOC_H */
>
Troy Lee Dec. 10, 2021, 2:33 p.m. UTC | #3
On Fri, Dec 10, 2021 at 10:05 PM Cédric Le Goater <clg@kaod.org> wrote:
>
> On 12/10/21 09:30, Troy Lee wrote:
> > AST2600 Display Port MCU introduces 0x18000000~0x1803FFFF as it's memory
> > and io address. If guest machine try to access DPMCU memory, it will
> > cause a fatal error.
>
> The Aspeed SoCs have an "aspeed_soc.io" region for unimplemented devices
> but it's too small. Anyhow, it is better to have per logic unit. We should
> change that one day.
>
Good idea!

> For my information, which FW image are you using ?
>

We're using Aspeed's SDK image, I tested with ast2600-default machine.
Prebuilt image can be download from:
https://github.com/AspeedTech-BMC/openbmc/releases/tag/v07.02

Without declaring the DPMCU memory, the image will hangs in u-boot.
We're still working on I3C and SPI issue to be resolved to get into rootfs.

Thanks for your review.
Troy Lee

> > Signed-off-by: Troy Lee <troy_lee@aspeedtech.com>
>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
>
> Thanks,
>
> C.
>
> > ---
> >   hw/arm/aspeed_ast2600.c     | 8 ++++++++
> >   include/hw/arm/aspeed_soc.h | 2 ++
> >   2 files changed, 10 insertions(+)
> >
> > diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> > index 0384357a95..e33483fb5d 100644
> > --- a/hw/arm/aspeed_ast2600.c
> > +++ b/hw/arm/aspeed_ast2600.c
> > @@ -19,9 +19,11 @@
> >   #include "sysemu/sysemu.h"
> >
> >   #define ASPEED_SOC_IOMEM_SIZE       0x00200000
> > +#define ASPEED_SOC_DPMCU_SIZE       0x00040000
> >
> >   static const hwaddr aspeed_soc_ast2600_memmap[] = {
> >       [ASPEED_DEV_SRAM]      = 0x10000000,
> > +    [ASPEED_DEV_DPMCU]     = 0x18000000,
> >       /* 0x16000000     0x17FFFFFF : AHB BUS do LPC Bus bridge */
> >       [ASPEED_DEV_IOMEM]     = 0x1E600000,
> >       [ASPEED_DEV_PWM]       = 0x1E610000,
> > @@ -44,6 +46,7 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = {
> >       [ASPEED_DEV_SCU]       = 0x1E6E2000,
> >       [ASPEED_DEV_XDMA]      = 0x1E6E7000,
> >       [ASPEED_DEV_ADC]       = 0x1E6E9000,
> > +    [ASPEED_DEV_DP]        = 0x1E6EB000,
> >       [ASPEED_DEV_VIDEO]     = 0x1E700000,
> >       [ASPEED_DEV_SDHCI]     = 0x1E740000,
> >       [ASPEED_DEV_EMMC]      = 0x1E750000,
> > @@ -104,6 +107,7 @@ static const int aspeed_soc_ast2600_irqmap[] = {
> >       [ASPEED_DEV_ETH3]      = 32,
> >       [ASPEED_DEV_ETH4]      = 33,
> >       [ASPEED_DEV_KCS]       = 138,   /* 138 -> 142 */
> > +    [ASPEED_DEV_DP]        = 62,
> >   };
> >
> >   static qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int ctrl)
> > @@ -298,6 +302,10 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
> >       memory_region_add_subregion(get_system_memory(),
> >                                   sc->memmap[ASPEED_DEV_SRAM], &s->sram);
> >
> > +    /* DPMCU */
> > +    create_unimplemented_device("aspeed.dpmcu", sc->memmap[ASPEED_DEV_DPMCU],
> > +                                ASPEED_SOC_DPMCU_SIZE);
> > +
> >       /* SCU */
> >       if (!sysbus_realize(SYS_BUS_DEVICE(&s->scu), errp)) {
> >           return;
> > diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> > index 8139358549..18fb7eed46 100644
> > --- a/include/hw/arm/aspeed_soc.h
> > +++ b/include/hw/arm/aspeed_soc.h
> > @@ -139,6 +139,8 @@ enum {
> >       ASPEED_DEV_EMMC,
> >       ASPEED_DEV_KCS,
> >       ASPEED_DEV_HACE,
> > +    ASPEED_DEV_DPMCU,
> > +    ASPEED_DEV_DP,
> >   };
> >
> >   #endif /* ASPEED_SOC_H */
> >
>
Cédric Le Goater Dec. 10, 2021, 3:13 p.m. UTC | #4
On 12/10/21 15:33, Troy Lee wrote:
> On Fri, Dec 10, 2021 at 10:05 PM Cédric Le Goater <clg@kaod.org> wrote:
>>
>> On 12/10/21 09:30, Troy Lee wrote:
>>> AST2600 Display Port MCU introduces 0x18000000~0x1803FFFF as it's memory
>>> and io address. If guest machine try to access DPMCU memory, it will
>>> cause a fatal error.
>>
>> The Aspeed SoCs have an "aspeed_soc.io" region for unimplemented devices
>> but it's too small. Anyhow, it is better to have per logic unit. We should
>> change that one day.
>>
> Good idea!
> 
>> For my information, which FW image are you using ?
>>
> 
> We're using Aspeed's SDK image, I tested with ast2600-default machine.
> Prebuilt image can be download from:
> https://github.com/AspeedTech-BMC/openbmc/releases/tag/v07.02

Excellent ! Is there one I could try in particular ?


Once correctly supported, we should include one of these SDK images in :

   tests/avocado/boot_linux_console.py

to complete our tests of the device models.

QEMU is not making much difference between the revision. You might need
to improve that.

> Without declaring the DPMCU memory, the image will hangs in u-boot.

yeah. You can use -d guest_errors,unimp to catch accesses done on AHB
windows not covered by the QEMU models. There are plenty of ways to
move past U-Boot when models are not implemented yet. Don't waste
too much time, just ask.

eMMC is only on these branches :

   https://github.com/openbmc/qemu/
   https://github.com/legoater/qemu/

Same for SBC and support is primitive.

> We're still working on I3C and SPI issue to be resolved to get into rootfs.

I3C has not much support in Linux and none in QEMU. You will have to
add dummy models.

SPI as a non-SPI flash driver ? The SPI flash controller models should
be quite well covered today. What's the issue ?

Thanks,

C.
Troy Lee Dec. 14, 2021, 7:57 a.m. UTC | #5
On Fri, Dec 10, 2021 at 11:13 PM Cédric Le Goater <clg@kaod.org> wrote:
>
> On 12/10/21 15:33, Troy Lee wrote:
> > On Fri, Dec 10, 2021 at 10:05 PM Cédric Le Goater <clg@kaod.org> wrote:
> >>
> >> On 12/10/21 09:30, Troy Lee wrote:
> >>> AST2600 Display Port MCU introduces 0x18000000~0x1803FFFF as it's memory
> >>> and io address. If guest machine try to access DPMCU memory, it will
> >>> cause a fatal error.
> >>
> >> The Aspeed SoCs have an "aspeed_soc.io" region for unimplemented devices
> >> but it's too small. Anyhow, it is better to have per logic unit. We should
> >> change that one day.
> >>
> > Good idea!
> >
> >> For my information, which FW image are you using ?
> >>
> >
> > We're using Aspeed's SDK image, I tested with ast2600-default machine.
> > Prebuilt image can be download from:
> > https://github.com/AspeedTech-BMC/openbmc/releases/tag/v07.02
>
> Excellent ! Is there one I could try in particular ?
You could use ast2600-default-obmc.tgz, but we will send another patch
for HACE engine.

>
>
> Once correctly supported, we should include one of these SDK images in :
>
>    tests/avocado/boot_linux_console.py
>
> to complete our tests of the device models.
Sure, once the image successfully boots into rootfs, I'll add a test
case for it.

> QEMU is not making much difference between the revision. You might need
> to improve that.
>
> > Without declaring the DPMCU memory, the image will hangs in u-boot.
>
> yeah. You can use -d guest_errors,unimp to catch accesses done on AHB
> windows not covered by the QEMU models. There are plenty of ways to
> move past U-Boot when models are not implemented yet. Don't waste
> too much time, just ask.
>
> eMMC is only on these branches :
>
>    https://github.com/openbmc/qemu/
>    https://github.com/legoater/qemu/
These two branches are very useful!

> Same for SBC and support is primitive.
>
> > We're still working on I3C and SPI issue to be resolved to get into rootfs.
>
> I3C has not much support in Linux and none in QEMU. You will have to
> add dummy models.
Can I submit a dummy model that only responds to the RESET control register?
Or it has to be well implemented like i2c/core.c and i2c/aspeed_i2c.c?


> SPI as a non-SPI flash driver ? The SPI flash controller models should
> be quite well covered today. What's the issue ?
I need some more investigation, because we're using a different spi
driver for fmc-spi.

>
> Thanks,
>
> C.

Thanks,
Troy Lee
Cédric Le Goater Dec. 14, 2021, 8:19 a.m. UTC | #6
On 12/14/21 08:57, Troy Lee wrote:
> On Fri, Dec 10, 2021 at 11:13 PM Cédric Le Goater <clg@kaod.org> wrote:
>>
>> On 12/10/21 15:33, Troy Lee wrote:
>>> On Fri, Dec 10, 2021 at 10:05 PM Cédric Le Goater <clg@kaod.org> wrote:
>>>>
>>>> On 12/10/21 09:30, Troy Lee wrote:
>>>>> AST2600 Display Port MCU introduces 0x18000000~0x1803FFFF as it's memory
>>>>> and io address. If guest machine try to access DPMCU memory, it will
>>>>> cause a fatal error.
>>>>
>>>> The Aspeed SoCs have an "aspeed_soc.io" region for unimplemented devices
>>>> but it's too small. Anyhow, it is better to have per logic unit. We should
>>>> change that one day.
>>>>
>>> Good idea!
>>>
>>>> For my information, which FW image are you using ?
>>>>
>>>
>>> We're using Aspeed's SDK image, I tested with ast2600-default machine.
>>> Prebuilt image can be download from:
>>> https://github.com/AspeedTech-BMC/openbmc/releases/tag/v07.02
>>
>> Excellent ! Is there one I could try in particular ?
> You could use ast2600-default-obmc.tgz, but we will send another patch
> for HACE engine.
> 
>>
>>
>> Once correctly supported, we should include one of these SDK images in :
>>
>>     tests/avocado/boot_linux_console.py
>>
>> to complete our tests of the device models.
> Sure, once the image successfully boots into rootfs, I'll add a test
> case for it.

yeah. No hurries.

>> QEMU is not making much difference between the revision. You might need
>> to improve that.
>>
>>> Without declaring the DPMCU memory, the image will hangs in u-boot.
>>
>> yeah. You can use -d guest_errors,unimp to catch accesses done on AHB
>> windows not covered by the QEMU models. There are plenty of ways to
>> move past U-Boot when models are not implemented yet. Don't waste
>> too much time, just ask.
>>
>> eMMC is only on these branches :
>>
>>     https://github.com/openbmc/qemu/
>>     https://github.com/legoater/qemu/
> These two branches are very useful!

One day, we should try to upstream eMMC support.
  
>> Same for SBC and support is primitive.
>>
>>> We're still working on I3C and SPI issue to be resolved to get into rootfs.
>>
>> I3C has not much support in Linux and none in QEMU. You will have to
>> add dummy models.
> Can I submit a dummy model that only responds to the RESET control register?

yes.

> Or it has to be well implemented like i2c/core.c and i2c/aspeed_i2c.c?

No it can be dummy to start with.

>> SPI as a non-SPI flash driver ? The SPI flash controller models should
>> be quite well covered today. What's the issue ?
> I need some more investigation, because we're using a different spi
> driver for fmc-spi.

Is that the one written by Chin-Ting Kuo in 2020 ? If so, I did send
some adjustment of the model for it, such as address lane disablement,
but there might be more to it.

Thanks,

C.
Peter Maydell Jan. 6, 2022, 5:41 p.m. UTC | #7
On Fri, 10 Dec 2021 at 14:05, Cédric Le Goater <clg@kaod.org> wrote:
>
> On 12/10/21 09:30, Troy Lee wrote:
> > AST2600 Display Port MCU introduces 0x18000000~0x1803FFFF as it's memory
> > and io address. If guest machine try to access DPMCU memory, it will
> > cause a fatal error.
>
> The Aspeed SoCs have an "aspeed_soc.io" region for unimplemented devices
> but it's too small. Anyhow, it is better to have per logic unit. We should
> change that one day.
>
> For my information, which FW image are you using ?
>
> > Signed-off-by: Troy Lee <troy_lee@aspeedtech.com>
>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>



Applied to target-arm.next, thanks.

-- PMM
diff mbox series

Patch

diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 0384357a95..e33483fb5d 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -19,9 +19,11 @@ 
 #include "sysemu/sysemu.h"
 
 #define ASPEED_SOC_IOMEM_SIZE       0x00200000
+#define ASPEED_SOC_DPMCU_SIZE       0x00040000
 
 static const hwaddr aspeed_soc_ast2600_memmap[] = {
     [ASPEED_DEV_SRAM]      = 0x10000000,
+    [ASPEED_DEV_DPMCU]     = 0x18000000,
     /* 0x16000000     0x17FFFFFF : AHB BUS do LPC Bus bridge */
     [ASPEED_DEV_IOMEM]     = 0x1E600000,
     [ASPEED_DEV_PWM]       = 0x1E610000,
@@ -44,6 +46,7 @@  static const hwaddr aspeed_soc_ast2600_memmap[] = {
     [ASPEED_DEV_SCU]       = 0x1E6E2000,
     [ASPEED_DEV_XDMA]      = 0x1E6E7000,
     [ASPEED_DEV_ADC]       = 0x1E6E9000,
+    [ASPEED_DEV_DP]        = 0x1E6EB000,
     [ASPEED_DEV_VIDEO]     = 0x1E700000,
     [ASPEED_DEV_SDHCI]     = 0x1E740000,
     [ASPEED_DEV_EMMC]      = 0x1E750000,
@@ -104,6 +107,7 @@  static const int aspeed_soc_ast2600_irqmap[] = {
     [ASPEED_DEV_ETH3]      = 32,
     [ASPEED_DEV_ETH4]      = 33,
     [ASPEED_DEV_KCS]       = 138,   /* 138 -> 142 */
+    [ASPEED_DEV_DP]        = 62,
 };
 
 static qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int ctrl)
@@ -298,6 +302,10 @@  static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     memory_region_add_subregion(get_system_memory(),
                                 sc->memmap[ASPEED_DEV_SRAM], &s->sram);
 
+    /* DPMCU */
+    create_unimplemented_device("aspeed.dpmcu", sc->memmap[ASPEED_DEV_DPMCU],
+                                ASPEED_SOC_DPMCU_SIZE);
+
     /* SCU */
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->scu), errp)) {
         return;
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 8139358549..18fb7eed46 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -139,6 +139,8 @@  enum {
     ASPEED_DEV_EMMC,
     ASPEED_DEV_KCS,
     ASPEED_DEV_HACE,
+    ASPEED_DEV_DPMCU,
+    ASPEED_DEV_DP,
 };
 
 #endif /* ASPEED_SOC_H */