| Message ID | 20170920070135.31379-2-clg@kaod.org |
|---|---|
| State | New |
| Headers | show |
| Series | aspeed: add a witherspoon-bmc machine | expand |
On 20 September 2017 at 08:01, Cédric Le Goater <clg@kaod.org> wrote: > The Witherspoon boards are OpenPOWER system hosting POWER9 Processors. > Let's add support for their BMC including a couple of I2C devices as > found on real HW. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > hw/arm/aspeed.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 49 insertions(+) > > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c > index ab895ad490af..81f522f711ae 100644 > --- a/hw/arm/aspeed.c > +++ b/hw/arm/aspeed.c > @@ -46,6 +46,7 @@ enum { > PALMETTO_BMC, > AST2500_EVB, > ROMULUS_BMC, > + WITHERSPOON_BMC, > }; > > /* Palmetto hardware value: 0x120CE416 */ > @@ -83,8 +84,12 @@ enum { > SCU_AST2500_HW_STRAP_ACPI_ENABLE | \ > SCU_HW_STRAP_SPI_MODE(SCU_HW_STRAP_SPI_MASTER)) > > +/* Witherspoon hardware value: 0xF10AD216 (but use romulus definition) */ > +#define WITHERSPOON_BMC_HW_STRAP1 ROMULUS_BMC_HW_STRAP1 > + > static void palmetto_bmc_i2c_init(AspeedBoardState *bmc); > static void ast2500_evb_i2c_init(AspeedBoardState *bmc); > +static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc); > > static const AspeedBoardConfig aspeed_boards[] = { > [PALMETTO_BMC] = { > @@ -110,6 +115,14 @@ static const AspeedBoardConfig aspeed_boards[] = { > .spi_model = "mx66l1g45g", > .num_cs = 2, > }, > + [WITHERSPOON_BMC] = { > + .soc_name = "ast2500-a1", > + .hw_strap1 = WITHERSPOON_BMC_HW_STRAP1, > + .fmc_model = "mx25l25635e", > + .spi_model = "mx66l1g45g", > + .num_cs = 2, > + .i2c_init = witherspoon_bmc_i2c_init, > + }, > }; > > #define FIRMWARE_ADDR 0x0 > @@ -337,11 +350,47 @@ static const TypeInfo romulus_bmc_type = { > .class_init = romulus_bmc_class_init, > }; > > +static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc) > +{ > + AspeedSoCState *soc = &bmc->soc; > + > + i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 4), "tmp423", 0x4c); > + i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 5), "tmp423", 0x4c); > + > + i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 9), "tmp105", 0x4a); > +} > + > +static void witherspoon_bmc_init(MachineState *machine) > +{ > + aspeed_board_init(machine, &aspeed_boards[WITHERSPOON_BMC]); > +} > + > +static void witherspoon_bmc_class_init(ObjectClass *oc, void *data) > +{ > + MachineClass *mc = MACHINE_CLASS(oc); > + > + mc->desc = "OpenPOWER Witherspoon BMC (ARM1176)"; > + mc->init = witherspoon_bmc_init; > + mc->max_cpus = 1; > + mc->no_sdcard = 1; > + mc->no_floppy = 1; > + mc->no_cdrom = 1; > + mc->no_parallel = 1; > + mc->ignore_memory_transaction_failures = true; Please don't set this flag for new board models, it is only for our legacy existing ones. Instead implement any devices that you need for guest code to boot (stub them out with create_unimplemented_device() if you like). thanks -- PMM
On 10/06/2017 05:10 PM, Peter Maydell wrote: > On 20 September 2017 at 08:01, Cédric Le Goater <clg@kaod.org> wrote: >> The Witherspoon boards are OpenPOWER system hosting POWER9 Processors. >> Let's add support for their BMC including a couple of I2C devices as >> found on real HW. >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> --- >> hw/arm/aspeed.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 49 insertions(+) >> >> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c >> index ab895ad490af..81f522f711ae 100644 >> --- a/hw/arm/aspeed.c >> +++ b/hw/arm/aspeed.c >> @@ -46,6 +46,7 @@ enum { >> PALMETTO_BMC, >> AST2500_EVB, >> ROMULUS_BMC, >> + WITHERSPOON_BMC, >> }; >> >> /* Palmetto hardware value: 0x120CE416 */ >> @@ -83,8 +84,12 @@ enum { >> SCU_AST2500_HW_STRAP_ACPI_ENABLE | \ >> SCU_HW_STRAP_SPI_MODE(SCU_HW_STRAP_SPI_MASTER)) >> >> +/* Witherspoon hardware value: 0xF10AD216 (but use romulus definition) */ >> +#define WITHERSPOON_BMC_HW_STRAP1 ROMULUS_BMC_HW_STRAP1 >> + >> static void palmetto_bmc_i2c_init(AspeedBoardState *bmc); >> static void ast2500_evb_i2c_init(AspeedBoardState *bmc); >> +static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc); >> >> static const AspeedBoardConfig aspeed_boards[] = { >> [PALMETTO_BMC] = { >> @@ -110,6 +115,14 @@ static const AspeedBoardConfig aspeed_boards[] = { >> .spi_model = "mx66l1g45g", >> .num_cs = 2, >> }, >> + [WITHERSPOON_BMC] = { >> + .soc_name = "ast2500-a1", >> + .hw_strap1 = WITHERSPOON_BMC_HW_STRAP1, >> + .fmc_model = "mx25l25635e", >> + .spi_model = "mx66l1g45g", >> + .num_cs = 2, >> + .i2c_init = witherspoon_bmc_i2c_init, >> + }, >> }; >> >> #define FIRMWARE_ADDR 0x0 >> @@ -337,11 +350,47 @@ static const TypeInfo romulus_bmc_type = { >> .class_init = romulus_bmc_class_init, >> }; >> >> +static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc) >> +{ >> + AspeedSoCState *soc = &bmc->soc; >> + >> + i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 4), "tmp423", 0x4c); >> + i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 5), "tmp423", 0x4c); >> + >> + i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 9), "tmp105", 0x4a); >> +} >> + >> +static void witherspoon_bmc_init(MachineState *machine) >> +{ >> + aspeed_board_init(machine, &aspeed_boards[WITHERSPOON_BMC]); >> +} >> + >> +static void witherspoon_bmc_class_init(ObjectClass *oc, void *data) >> +{ >> + MachineClass *mc = MACHINE_CLASS(oc); >> + >> + mc->desc = "OpenPOWER Witherspoon BMC (ARM1176)"; >> + mc->init = witherspoon_bmc_init; >> + mc->max_cpus = 1; >> + mc->no_sdcard = 1; >> + mc->no_floppy = 1; >> + mc->no_cdrom = 1; >> + mc->no_parallel = 1; >> + mc->ignore_memory_transaction_failures = true; > > Please don't set this flag for new board models, it is only > for our legacy existing ones. Instead implement any devices > that you need for guest code to boot (stub them out with > create_unimplemented_device() if you like). OK. I am discovering this. I will take a look for the next round. C.
On Wed, 2017-09-20 at 09:01 +0200, Cédric Le Goater wrote: > The Witherspoon boards are OpenPOWER system hosting POWER9 Processors. > Let's add support for their BMC including a couple of I2C devices as > found on real HW. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > hw/arm/aspeed.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 49 insertions(+) > > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c > index ab895ad490af..81f522f711ae 100644 > --- a/hw/arm/aspeed.c > +++ b/hw/arm/aspeed.c > @@ -46,6 +46,7 @@ enum { > PALMETTO_BMC, > AST2500_EVB, > ROMULUS_BMC, > + WITHERSPOON_BMC, > }; > > /* Palmetto hardware value: 0x120CE416 */ > @@ -83,8 +84,12 @@ enum { > SCU_AST2500_HW_STRAP_ACPI_ENABLE | \ > SCU_HW_STRAP_SPI_MODE(SCU_HW_STRAP_SPI_MASTER)) > > +/* Witherspoon hardware value: 0xF10AD216 (but use romulus definition) */ > +#define WITHERSPOON_BMC_HW_STRAP1 ROMULUS_BMC_HW_STRAP1 > + > static void palmetto_bmc_i2c_init(AspeedBoardState *bmc); > static void ast2500_evb_i2c_init(AspeedBoardState *bmc); > +static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc); > > static const AspeedBoardConfig aspeed_boards[] = { > [PALMETTO_BMC] = { > @@ -110,6 +115,14 @@ static const AspeedBoardConfig aspeed_boards[] = { > .spi_model = "mx66l1g45g", > .num_cs = 2, > }, > + [WITHERSPOON_BMC] = { > + .soc_name = "ast2500-a1", > + .hw_strap1 = WITHERSPOON_BMC_HW_STRAP1, > + .fmc_model = "mx25l25635e", > + .spi_model = "mx66l1g45g", > + .num_cs = 2, > + .i2c_init = witherspoon_bmc_i2c_init, > + }, > }; > > #define FIRMWARE_ADDR 0x0 > @@ -337,11 +350,47 @@ static const TypeInfo romulus_bmc_type = { > .class_init = romulus_bmc_class_init, > }; > > +static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc) > +{ > + AspeedSoCState *soc = &bmc->soc; > + > + i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 4), "tmp423", 0x4c); > + i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 5), "tmp423", 0x4c); > + > + i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 9), "tmp105", 0x4a); Looks like I need to track down newer versions of the schematics I have. > +} > + > +static void witherspoon_bmc_init(MachineState *machine) > +{ > + aspeed_board_init(machine, &aspeed_boards[WITHERSPOON_BMC]); > +} > + > +static void witherspoon_bmc_class_init(ObjectClass *oc, void *data) > +{ > + MachineClass *mc = MACHINE_CLASS(oc); > + > + mc->desc = "OpenPOWER Witherspoon BMC (ARM1176)"; > + mc->init = witherspoon_bmc_init; > + mc->max_cpus = 1; > + mc->no_sdcard = 1; > + mc->no_floppy = 1; > + mc->no_cdrom = 1; > + mc->no_parallel = 1; > + mc->ignore_memory_transaction_failures = true; Aside from the issue with the above as pointed out by Peter, Reviewed-by: Andrew Jeffery <andrew@aj.id.au> > +} > + > +static const TypeInfo witherspoon_bmc_type = { > + .name = MACHINE_TYPE_NAME("witherspoon-bmc"), > + .parent = TYPE_MACHINE, > + .class_init = witherspoon_bmc_class_init, > +}; > + > static void aspeed_machine_init(void) > { > type_register_static(&palmetto_bmc_type); > type_register_static(&ast2500_evb_type); > type_register_static(&romulus_bmc_type); > + type_register_static(&witherspoon_bmc_type); > } > > type_init(aspeed_machine_init)
On 10/06/2017 05:10 PM, Peter Maydell wrote: > On 20 September 2017 at 08:01, Cédric Le Goater <clg@kaod.org> wrote: >> The Witherspoon boards are OpenPOWER system hosting POWER9 Processors. >> Let's add support for their BMC including a couple of I2C devices as >> found on real HW. >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> --- >> hw/arm/aspeed.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 49 insertions(+) >> >> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c >> index ab895ad490af..81f522f711ae 100644 >> --- a/hw/arm/aspeed.c >> +++ b/hw/arm/aspeed.c >> @@ -46,6 +46,7 @@ enum { >> PALMETTO_BMC, >> AST2500_EVB, >> ROMULUS_BMC, >> + WITHERSPOON_BMC, >> }; >> >> /* Palmetto hardware value: 0x120CE416 */ >> @@ -83,8 +84,12 @@ enum { >> SCU_AST2500_HW_STRAP_ACPI_ENABLE | \ >> SCU_HW_STRAP_SPI_MODE(SCU_HW_STRAP_SPI_MASTER)) >> >> +/* Witherspoon hardware value: 0xF10AD216 (but use romulus definition) */ >> +#define WITHERSPOON_BMC_HW_STRAP1 ROMULUS_BMC_HW_STRAP1 >> + >> static void palmetto_bmc_i2c_init(AspeedBoardState *bmc); >> static void ast2500_evb_i2c_init(AspeedBoardState *bmc); >> +static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc); >> >> static const AspeedBoardConfig aspeed_boards[] = { >> [PALMETTO_BMC] = { >> @@ -110,6 +115,14 @@ static const AspeedBoardConfig aspeed_boards[] = { >> .spi_model = "mx66l1g45g", >> .num_cs = 2, >> }, >> + [WITHERSPOON_BMC] = { >> + .soc_name = "ast2500-a1", >> + .hw_strap1 = WITHERSPOON_BMC_HW_STRAP1, >> + .fmc_model = "mx25l25635e", >> + .spi_model = "mx66l1g45g", >> + .num_cs = 2, >> + .i2c_init = witherspoon_bmc_i2c_init, >> + }, >> }; >> >> #define FIRMWARE_ADDR 0x0 >> @@ -337,11 +350,47 @@ static const TypeInfo romulus_bmc_type = { >> .class_init = romulus_bmc_class_init, >> }; >> >> +static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc) >> +{ >> + AspeedSoCState *soc = &bmc->soc; >> + >> + i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 4), "tmp423", 0x4c); >> + i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 5), "tmp423", 0x4c); >> + >> + i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 9), "tmp105", 0x4a); >> +} >> + >> +static void witherspoon_bmc_init(MachineState *machine) >> +{ >> + aspeed_board_init(machine, &aspeed_boards[WITHERSPOON_BMC]); >> +} >> + >> +static void witherspoon_bmc_class_init(ObjectClass *oc, void *data) >> +{ >> + MachineClass *mc = MACHINE_CLASS(oc); >> + >> + mc->desc = "OpenPOWER Witherspoon BMC (ARM1176)"; >> + mc->init = witherspoon_bmc_init; >> + mc->max_cpus = 1; >> + mc->no_sdcard = 1; >> + mc->no_floppy = 1; >> + mc->no_cdrom = 1; >> + mc->no_parallel = 1; >> + mc->ignore_memory_transaction_failures = true; > > Please don't set this flag for new board models, it is only > for our legacy existing ones. Instead implement any devices > that you need for guest code to boot (stub them out with > create_unimplemented_device() if you like). I have dug into this a little more and it seems that it is required for the Aspeed bootloader (a modified u-boot) which uses static variables in early init phases. So legacy firmwares won't work in QEMU but will on real HW. It's fine with me but what is the goal of the approach ? Force SoC providers into fixing their FW when they use QEMU ? Thanks, C.
On 10 October 2017 at 10:19, Cédric Le Goater <clg@kaod.org> wrote: > On 10/06/2017 05:10 PM, Peter Maydell wrote: >>> + mc->ignore_memory_transaction_failures = true; >> >> Please don't set this flag for new board models, it is only >> for our legacy existing ones. Instead implement any devices >> that you need for guest code to boot (stub them out with >> create_unimplemented_device() if you like). > > I have dug into this a little more and it seems that it is > required for the Aspeed bootloader (a modified u-boot) which > uses static variables in early init phases. So legacy firmwares > won't work in QEMU but will on real HW. > > It's fine with me but what is the goal of the approach ? Force > SoC providers into fixing their FW when they use QEMU ? The goal is to model hardware correctly. Hardware gives aborts if you touch a physical address with no device there, and so QEMU's model should do the same. If you have guest code that touches a physical address and blows up because of an abort (but doesn't when run on h/w) then either: * it is trying to probe a device that exists in real h/w: you need to provide a stub implementation in QEMU * the SoC's bus fabric really doesn't pass aborts back to the CPU; I think this is unlikely, but you can model it at the SoC level with a suitable default memory region The purpose of the flag is purely for existing board models, where it is impossible to enable the correct (abort) behaviour without possibly breaking guest code images that work for people using released QEMU code. On a new board model we don't have that problem and we can get it right from the start. If we don't get it right from the start then we will never have a chance to fix it in future. Our mismodelling of this (not turning accesses to invalid addresses into CPU aborts) meant that in the past it was possible to be lazy when implementing a board model and just not model half the hardware at all. Now it isn't, but I don't think that adding a set of calls to create_unimplemented_device() is a significant imposition. thanks -- PMM
On 10/10/2017 11:54 AM, Peter Maydell wrote: > On 10 October 2017 at 10:19, Cédric Le Goater <clg@kaod.org> wrote: >> On 10/06/2017 05:10 PM, Peter Maydell wrote: >>>> + mc->ignore_memory_transaction_failures = true; >>> >>> Please don't set this flag for new board models, it is only >>> for our legacy existing ones. Instead implement any devices >>> that you need for guest code to boot (stub them out with >>> create_unimplemented_device() if you like). >> >> I have dug into this a little more and it seems that it is >> required for the Aspeed bootloader (a modified u-boot) which >> uses static variables in early init phases. So legacy firmwares >> won't work in QEMU but will on real HW. >> >> It's fine with me but what is the goal of the approach ? Force >> SoC providers into fixing their FW when they use QEMU ? > > The goal is to model hardware correctly. Hardware gives > aborts if you touch a physical address with no device there, > and so QEMU's model should do the same. If you have guest > code that touches a physical address and blows up because > of an abort (but doesn't when run on h/w) then either: > * it is trying to probe a device that exists in real h/w: > you need to provide a stub implementation in QEMU > * the SoC's bus fabric really doesn't pass aborts back > to the CPU; I think this is unlikely, but you can model > it at the SoC level with a suitable default memory region well, that is case it seems. Anyhow, I found the required fixes in u-boot, so I will go with ignore_memory_transaction_failures=false with this new machine. Thanks, C. > The purpose of the flag is purely for existing board models, > where it is impossible to enable the correct (abort) > behaviour without possibly breaking guest code images that > work for people using released QEMU code. On a new board > model we don't have that problem and we can get it right > from the start. If we don't get it right from the start > then we will never have a chance to fix it in future. > > Our mismodelling of this (not turning accesses to invalid > addresses into CPU aborts) meant that in the past it was > possible to be lazy when implementing a board model and > just not model half the hardware at all. Now it isn't, > but I don't think that adding a set of calls to > create_unimplemented_device() is a significant imposition. > > thanks > -- PMM >
On 10 October 2017 at 14:21, Cédric Le Goater <clg@kaod.org> wrote: > On 10/10/2017 11:54 AM, Peter Maydell wrote: >> The goal is to model hardware correctly. Hardware gives >> aborts if you touch a physical address with no device there, >> and so QEMU's model should do the same. If you have guest >> code that touches a physical address and blows up because >> of an abort (but doesn't when run on h/w) then either: >> * it is trying to probe a device that exists in real h/w: >> you need to provide a stub implementation in QEMU >> * the SoC's bus fabric really doesn't pass aborts back >> to the CPU; I think this is unlikely, but you can model >> it at the SoC level with a suitable default memory region > > well, that is case it seems. If it is, then we should model the SoC that way, ie find out from the hardware docs what part of the bus fabric ignores decode errors and use memory regions with the right default behaviour to cover the relevant address ranges. thanks -- PMM
On 10/09/2017 02:04 AM, Andrew Jeffery wrote: > On Wed, 2017-09-20 at 09:01 +0200, Cédric Le Goater wrote: >> The Witherspoon boards are OpenPOWER system hosting POWER9 Processors. >> Let's add support for their BMC including a couple of I2C devices as >> found on real HW. >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> --- >> hw/arm/aspeed.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 49 insertions(+) >> >> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c >> index ab895ad490af..81f522f711ae 100644 >> --- a/hw/arm/aspeed.c >> +++ b/hw/arm/aspeed.c >> @@ -46,6 +46,7 @@ enum { >> PALMETTO_BMC, >> AST2500_EVB, >> ROMULUS_BMC, >> + WITHERSPOON_BMC, >> }; >> >> /* Palmetto hardware value: 0x120CE416 */ >> @@ -83,8 +84,12 @@ enum { >> SCU_AST2500_HW_STRAP_ACPI_ENABLE | \ >> SCU_HW_STRAP_SPI_MODE(SCU_HW_STRAP_SPI_MASTER)) >> >> +/* Witherspoon hardware value: 0xF10AD216 (but use romulus definition) */ >> +#define WITHERSPOON_BMC_HW_STRAP1 ROMULUS_BMC_HW_STRAP1 >> + >> static void palmetto_bmc_i2c_init(AspeedBoardState *bmc); >> static void ast2500_evb_i2c_init(AspeedBoardState *bmc); >> +static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc); >> >> static const AspeedBoardConfig aspeed_boards[] = { >> [PALMETTO_BMC] = { >> @@ -110,6 +115,14 @@ static const AspeedBoardConfig aspeed_boards[] = { >> .spi_model = "mx66l1g45g", >> .num_cs = 2, >> }, >> + [WITHERSPOON_BMC] = { >> + .soc_name = "ast2500-a1", >> + .hw_strap1 = WITHERSPOON_BMC_HW_STRAP1, >> + .fmc_model = "mx25l25635e", >> + .spi_model = "mx66l1g45g", >> + .num_cs = 2, >> + .i2c_init = witherspoon_bmc_i2c_init, >> + }, >> }; >> >> #define FIRMWARE_ADDR 0x0 >> @@ -337,11 +350,47 @@ static const TypeInfo romulus_bmc_type = { >> .class_init = romulus_bmc_class_init, >> }; >> >> +static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc) >> +{ >> + AspeedSoCState *soc = &bmc->soc; >> + >> + i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 4), "tmp423", 0x4c); >> + i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 5), "tmp423", 0x4c); >> + >> + i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 9), "tmp105", 0x4a); > > Looks like I need to track down newer versions of the schematics I have. the device on the board is a tmp275 but the tmp105 model is compatible. >> +} >> + >> +static void witherspoon_bmc_init(MachineState *machine) >> +{ >> + aspeed_board_init(machine, &aspeed_boards[WITHERSPOON_BMC]); >> +} >> + >> +static void witherspoon_bmc_class_init(ObjectClass *oc, void *data) >> +{ >> + MachineClass *mc = MACHINE_CLASS(oc); >> + >> + mc->desc = "OpenPOWER Witherspoon BMC (ARM1176)"; >> + mc->init = witherspoon_bmc_init; >> + mc->max_cpus = 1; >> + mc->no_sdcard = 1; >> + mc->no_floppy = 1; >> + mc->no_cdrom = 1; >> + mc->no_parallel = 1; >> + mc->ignore_memory_transaction_failures = true; > > Aside from the issue with the above as pointed out by Peter, > > Reviewed-by: Andrew Jeffery <andrew@aj.id.au> Thanks, C. > >> +} >> + >> +static const TypeInfo witherspoon_bmc_type = { >> + .name = MACHINE_TYPE_NAME("witherspoon-bmc"), >> + .parent = TYPE_MACHINE, >> + .class_init = witherspoon_bmc_class_init, >> +}; >> + >> static void aspeed_machine_init(void) >> { >> type_register_static(&palmetto_bmc_type); >> type_register_static(&ast2500_evb_type); >> type_register_static(&romulus_bmc_type); >> + type_register_static(&witherspoon_bmc_type); >> } >> >> type_init(aspeed_machine_init)
On 10 October 2017 at 14:30, Cédric Le Goater <clg@kaod.org> wrote: > On 10/09/2017 02:04 AM, Andrew Jeffery wrote: >> On Wed, 2017-09-20 at 09:01 +0200, Cédric Le Goater wrote: >>> +static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc) >>> +{ >>> + AspeedSoCState *soc = &bmc->soc; >>> + >>> + i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 4), "tmp423", 0x4c); >>> + i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 5), "tmp423", 0x4c); >>> + >>> + i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 9), "tmp105", 0x4a); >> >> Looks like I need to track down newer versions of the schematics I have. > > the device on the board is a tmp275 but the tmp105 model is compatible. This kind of deviation from the real hardware is worth having a comment to document, I think. thanks -- PMM
On 10/10/2017 03:24 PM, Peter Maydell wrote: > On 10 October 2017 at 14:21, Cédric Le Goater <clg@kaod.org> wrote: >> On 10/10/2017 11:54 AM, Peter Maydell wrote: >>> The goal is to model hardware correctly. Hardware gives >>> aborts if you touch a physical address with no device there, >>> and so QEMU's model should do the same. If you have guest >>> code that touches a physical address and blows up because >>> of an abort (but doesn't when run on h/w) then either: >>> * it is trying to probe a device that exists in real h/w: >>> you need to provide a stub implementation in QEMU >>> * the SoC's bus fabric really doesn't pass aborts back >>> to the CPU; I think this is unlikely, but you can model >>> it at the SoC level with a suitable default memory region >> >> well, that is case it seems. > > If it is, then we should model the SoC that way, ie find > out from the hardware docs what part of the bus fabric > ignores decode errors and use memory regions with the > right default behaviour to cover the relevant address > ranges. The addresses generating memory fault errors are all in the region where the BMC SPI Flash Memory is mapped : [ 20000000-2FFFFFFF ] but we should not be doing any writes there. I will make some inquiries. Thanks, C.
On 10 October 2017 at 16:38, Cédric Le Goater <clg@kaod.org> wrote: > On 10/10/2017 03:24 PM, Peter Maydell wrote: >> On 10 October 2017 at 14:21, Cédric Le Goater <clg@kaod.org> wrote: >>> On 10/10/2017 11:54 AM, Peter Maydell wrote: >>>> The goal is to model hardware correctly. Hardware gives >>>> aborts if you touch a physical address with no device there, >>>> and so QEMU's model should do the same. If you have guest >>>> code that touches a physical address and blows up because >>>> of an abort (but doesn't when run on h/w) then either: >>>> * it is trying to probe a device that exists in real h/w: >>>> you need to provide a stub implementation in QEMU >>>> * the SoC's bus fabric really doesn't pass aborts back >>>> to the CPU; I think this is unlikely, but you can model >>>> it at the SoC level with a suitable default memory region >>> >>> well, that is case it seems. >> >> If it is, then we should model the SoC that way, ie find >> out from the hardware docs what part of the bus fabric >> ignores decode errors and use memory regions with the >> right default behaviour to cover the relevant address >> ranges. > > The addresses generating memory fault errors are all in > the region where the BMC SPI Flash Memory is mapped : > [ 20000000-2FFFFFFF ] If there's an actual flash device there then this sounds like my first case above, where we just need to stub out that range of addresses until we get round to really implementing the flash device. thanks -- PMM
On 10/10/2017 05:45 PM, Peter Maydell wrote: > On 10 October 2017 at 16:38, Cédric Le Goater <clg@kaod.org> wrote: >> On 10/10/2017 03:24 PM, Peter Maydell wrote: >>> On 10 October 2017 at 14:21, Cédric Le Goater <clg@kaod.org> wrote: >>>> On 10/10/2017 11:54 AM, Peter Maydell wrote: >>>>> The goal is to model hardware correctly. Hardware gives >>>>> aborts if you touch a physical address with no device there, >>>>> and so QEMU's model should do the same. If you have guest >>>>> code that touches a physical address and blows up because >>>>> of an abort (but doesn't when run on h/w) then either: >>>>> * it is trying to probe a device that exists in real h/w: >>>>> you need to provide a stub implementation in QEMU >>>>> * the SoC's bus fabric really doesn't pass aborts back >>>>> to the CPU; I think this is unlikely, but you can model >>>>> it at the SoC level with a suitable default memory region >>>> >>>> well, that is case it seems. >>> >>> If it is, then we should model the SoC that way, ie find >>> out from the hardware docs what part of the bus fabric >>> ignores decode errors and use memory regions with the >>> right default behaviour to cover the relevant address >>> ranges. >> >> The addresses generating memory fault errors are all in >> the region where the BMC SPI Flash Memory is mapped : >> [ 20000000-2FFFFFFF ] > > If there's an actual flash device there then this sounds > like my first case above, where we just need to stub out > that range of addresses until we get round to really > implementing the flash device. but it is implemented ! and the region available. C.
On Tue, 2017-10-10 at 15:30 +0200, Cédric Le Goater wrote: > On 10/09/2017 02:04 AM, Andrew Jeffery wrote: > > On Wed, 2017-09-20 at 09:01 +0200, Cédric Le Goater wrote: > > > The Witherspoon boards are OpenPOWER system hosting POWER9 Processors. > > > Let's add support for their BMC including a couple of I2C devices as > > > found on real HW. > > > > > > > > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > > > --- > > > hw/arm/aspeed.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 49 insertions(+) > > > > > > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c > > > index ab895ad490af..81f522f711ae 100644 > > > --- a/hw/arm/aspeed.c > > > +++ b/hw/arm/aspeed.c > > > @@ -46,6 +46,7 @@ enum { > > > PALMETTO_BMC, > > > AST2500_EVB, > > > ROMULUS_BMC, > > > + WITHERSPOON_BMC, > > > }; > > > > > > /* Palmetto hardware value: 0x120CE416 */ > > > @@ -83,8 +84,12 @@ enum { > > > SCU_AST2500_HW_STRAP_ACPI_ENABLE | \ > > > SCU_HW_STRAP_SPI_MODE(SCU_HW_STRAP_SPI_MASTER)) > > > > > > +/* Witherspoon hardware value: 0xF10AD216 (but use romulus definition) */ > > > +#define WITHERSPOON_BMC_HW_STRAP1 ROMULUS_BMC_HW_STRAP1 > > > + > > > static void palmetto_bmc_i2c_init(AspeedBoardState *bmc); > > > static void ast2500_evb_i2c_init(AspeedBoardState *bmc); > > > +static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc); > > > > > > static const AspeedBoardConfig aspeed_boards[] = { > > > [PALMETTO_BMC] = { > > > @@ -110,6 +115,14 @@ static const AspeedBoardConfig aspeed_boards[] = { > > > .spi_model = "mx66l1g45g", > > > .num_cs = 2, > > > }, > > > + [WITHERSPOON_BMC] = { > > > + .soc_name = "ast2500-a1", > > > + .hw_strap1 = WITHERSPOON_BMC_HW_STRAP1, > > > + .fmc_model = "mx25l25635e", > > > + .spi_model = "mx66l1g45g", > > > + .num_cs = 2, > > > + .i2c_init = witherspoon_bmc_i2c_init, > > > + }, > > > }; > > > > > > #define FIRMWARE_ADDR 0x0 > > > @@ -337,11 +350,47 @@ static const TypeInfo romulus_bmc_type = { > > > .class_init = romulus_bmc_class_init, > > > }; > > > > > > +static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc) > > > +{ > > > + AspeedSoCState *soc = &bmc->soc; > > > + > > > + i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 4), "tmp423", 0x4c); > > > + i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 5), "tmp423", 0x4c); > > > + > > > + i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 9), "tmp105", 0x4a); > > > > Looks like I need to track down newer versions of the schematics I have. > > the device on the board is a tmp275 but the tmp105 model is compatible. It neither device is listed in the version I have :) Andrew
On 10/11/2017 05:49 AM, Andrew Jeffery wrote: > On Tue, 2017-10-10 at 15:30 +0200, Cédric Le Goater wrote: >> On 10/09/2017 02:04 AM, Andrew Jeffery wrote: >>> On Wed, 2017-09-20 at 09:01 +0200, Cédric Le Goater wrote: >>>> The Witherspoon boards are OpenPOWER system hosting POWER9 Processors. >>>> Let's add support for their BMC including a couple of I2C devices as >>>> found on real HW. >>>> >>>>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org> >>>> --- >>>> hw/arm/aspeed.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ >>>> 1 file changed, 49 insertions(+) >>>> >>>> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c >>>> index ab895ad490af..81f522f711ae 100644 >>>> --- a/hw/arm/aspeed.c >>>> +++ b/hw/arm/aspeed.c >>>> @@ -46,6 +46,7 @@ enum { >>>> PALMETTO_BMC, >>>> AST2500_EVB, >>>> ROMULUS_BMC, >>>> + WITHERSPOON_BMC, >>>> }; >>>> >>>> /* Palmetto hardware value: 0x120CE416 */ >>>> @@ -83,8 +84,12 @@ enum { >>>> SCU_AST2500_HW_STRAP_ACPI_ENABLE | \ >>>> SCU_HW_STRAP_SPI_MODE(SCU_HW_STRAP_SPI_MASTER)) >>>> >>>> +/* Witherspoon hardware value: 0xF10AD216 (but use romulus definition) */ >>>> +#define WITHERSPOON_BMC_HW_STRAP1 ROMULUS_BMC_HW_STRAP1 >>>> + >>>> static void palmetto_bmc_i2c_init(AspeedBoardState *bmc); >>>> static void ast2500_evb_i2c_init(AspeedBoardState *bmc); >>>> +static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc); >>>> >>>> static const AspeedBoardConfig aspeed_boards[] = { >>>> [PALMETTO_BMC] = { >>>> @@ -110,6 +115,14 @@ static const AspeedBoardConfig aspeed_boards[] = { >>>> .spi_model = "mx66l1g45g", >>>> .num_cs = 2, >>>> }, >>>> + [WITHERSPOON_BMC] = { >>>> + .soc_name = "ast2500-a1", >>>> + .hw_strap1 = WITHERSPOON_BMC_HW_STRAP1, >>>> + .fmc_model = "mx25l25635e", >>>> + .spi_model = "mx66l1g45g", >>>> + .num_cs = 2, >>>> + .i2c_init = witherspoon_bmc_i2c_init, >>>> + }, >>>> }; >>>> >>>> #define FIRMWARE_ADDR 0x0 >>>> @@ -337,11 +350,47 @@ static const TypeInfo romulus_bmc_type = { >>>> .class_init = romulus_bmc_class_init, >>>> }; >>>> >>>> +static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc) >>>> +{ >>>> + AspeedSoCState *soc = &bmc->soc; >>>> + >>>> + i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 4), "tmp423", 0x4c); >>>> + i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 5), "tmp423", 0x4c); >>>> + >>>> + i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 9), "tmp105", 0x4a); >>> >>> Looks like I need to track down newer versions of the schematics I have. >> >> the device on the board is a tmp275 but the tmp105 model is compatible. > > It neither device is listed in the version I have :) Here is my source : https://github.com/openbmc/linux/blob/dev-4.10/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts#L504 C.
On Wed, 2017-10-11 at 09:28 +0200, Cédric Le Goater wrote: > On 10/11/2017 05:49 AM, Andrew Jeffery wrote: > > On Tue, 2017-10-10 at 15:30 +0200, Cédric Le Goater wrote: > > > On 10/09/2017 02:04 AM, Andrew Jeffery wrote: > > > > On Wed, 2017-09-20 at 09:01 +0200, Cédric Le Goater wrote: > > > > > The Witherspoon boards are OpenPOWER system hosting POWER9 Processors. > > > > > Let's add support for their BMC including a couple of I2C devices as > > > > > found on real HW. > > > > > > > > > > > > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > > > > > > > > > > --- > > > > > hw/arm/aspeed.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ > > > > > 1 file changed, 49 insertions(+) > > > > > > > > > > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c > > > > > index ab895ad490af..81f522f711ae 100644 > > > > > --- a/hw/arm/aspeed.c > > > > > +++ b/hw/arm/aspeed.c > > > > > @@ -46,6 +46,7 @@ enum { > > > > > PALMETTO_BMC, > > > > > AST2500_EVB, > > > > > ROMULUS_BMC, > > > > > + WITHERSPOON_BMC, > > > > > }; > > > > > > > > > > /* Palmetto hardware value: 0x120CE416 */ > > > > > @@ -83,8 +84,12 @@ enum { > > > > > SCU_AST2500_HW_STRAP_ACPI_ENABLE | \ > > > > > SCU_HW_STRAP_SPI_MODE(SCU_HW_STRAP_SPI_MASTER)) > > > > > > > > > > +/* Witherspoon hardware value: 0xF10AD216 (but use romulus definition) */ > > > > > +#define WITHERSPOON_BMC_HW_STRAP1 ROMULUS_BMC_HW_STRAP1 > > > > > + > > > > > static void palmetto_bmc_i2c_init(AspeedBoardState *bmc); > > > > > static void ast2500_evb_i2c_init(AspeedBoardState *bmc); > > > > > +static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc); > > > > > > > > > > static const AspeedBoardConfig aspeed_boards[] = { > > > > > [PALMETTO_BMC] = { > > > > > @@ -110,6 +115,14 @@ static const AspeedBoardConfig aspeed_boards[] = { > > > > > .spi_model = "mx66l1g45g", > > > > > .num_cs = 2, > > > > > }, > > > > > + [WITHERSPOON_BMC] = { > > > > > + .soc_name = "ast2500-a1", > > > > > + .hw_strap1 = WITHERSPOON_BMC_HW_STRAP1, > > > > > + .fmc_model = "mx25l25635e", > > > > > + .spi_model = "mx66l1g45g", > > > > > + .num_cs = 2, > > > > > + .i2c_init = witherspoon_bmc_i2c_init, > > > > > + }, > > > > > }; > > > > > > > > > > #define FIRMWARE_ADDR 0x0 > > > > > @@ -337,11 +350,47 @@ static const TypeInfo romulus_bmc_type = { > > > > > .class_init = romulus_bmc_class_init, > > > > > }; > > > > > > > > > > +static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc) > > > > > +{ > > > > > + AspeedSoCState *soc = &bmc->soc; > > > > > + > > > > > + i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 4), "tmp423", 0x4c); > > > > > + i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 5), "tmp423", 0x4c); > > > > > + > > > > > + i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 9), "tmp105", 0x4a); > > > > > > > > Looks like I need to track down newer versions of the schematics I have. > > > > > > the device on the board is a tmp275 but the tmp105 model is compatible. > > > > It neither device is listed in the version I have :) > > Here is my source : > > https://github.com/openbmc/linux/blob/dev-4.10/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts#L504 > Yeah, I ended up jumping on a machine and verifying the device was on the bus. Cheers, Andrew
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index ab895ad490af..81f522f711ae 100644 --- a/hw/arm/aspeed.c +++ b/hw/arm/aspeed.c @@ -46,6 +46,7 @@ enum { PALMETTO_BMC, AST2500_EVB, ROMULUS_BMC, + WITHERSPOON_BMC, }; /* Palmetto hardware value: 0x120CE416 */ @@ -83,8 +84,12 @@ enum { SCU_AST2500_HW_STRAP_ACPI_ENABLE | \ SCU_HW_STRAP_SPI_MODE(SCU_HW_STRAP_SPI_MASTER)) +/* Witherspoon hardware value: 0xF10AD216 (but use romulus definition) */ +#define WITHERSPOON_BMC_HW_STRAP1 ROMULUS_BMC_HW_STRAP1 + static void palmetto_bmc_i2c_init(AspeedBoardState *bmc); static void ast2500_evb_i2c_init(AspeedBoardState *bmc); +static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc); static const AspeedBoardConfig aspeed_boards[] = { [PALMETTO_BMC] = { @@ -110,6 +115,14 @@ static const AspeedBoardConfig aspeed_boards[] = { .spi_model = "mx66l1g45g", .num_cs = 2, }, + [WITHERSPOON_BMC] = { + .soc_name = "ast2500-a1", + .hw_strap1 = WITHERSPOON_BMC_HW_STRAP1, + .fmc_model = "mx25l25635e", + .spi_model = "mx66l1g45g", + .num_cs = 2, + .i2c_init = witherspoon_bmc_i2c_init, + }, }; #define FIRMWARE_ADDR 0x0 @@ -337,11 +350,47 @@ static const TypeInfo romulus_bmc_type = { .class_init = romulus_bmc_class_init, }; +static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc) +{ + AspeedSoCState *soc = &bmc->soc; + + i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 4), "tmp423", 0x4c); + i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 5), "tmp423", 0x4c); + + i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 9), "tmp105", 0x4a); +} + +static void witherspoon_bmc_init(MachineState *machine) +{ + aspeed_board_init(machine, &aspeed_boards[WITHERSPOON_BMC]); +} + +static void witherspoon_bmc_class_init(ObjectClass *oc, void *data) +{ + MachineClass *mc = MACHINE_CLASS(oc); + + mc->desc = "OpenPOWER Witherspoon BMC (ARM1176)"; + mc->init = witherspoon_bmc_init; + mc->max_cpus = 1; + mc->no_sdcard = 1; + mc->no_floppy = 1; + mc->no_cdrom = 1; + mc->no_parallel = 1; + mc->ignore_memory_transaction_failures = true; +} + +static const TypeInfo witherspoon_bmc_type = { + .name = MACHINE_TYPE_NAME("witherspoon-bmc"), + .parent = TYPE_MACHINE, + .class_init = witherspoon_bmc_class_init, +}; + static void aspeed_machine_init(void) { type_register_static(&palmetto_bmc_type); type_register_static(&ast2500_evb_type); type_register_static(&romulus_bmc_type); + type_register_static(&witherspoon_bmc_type); } type_init(aspeed_machine_init)
The Witherspoon boards are OpenPOWER system hosting POWER9 Processors. Let's add support for their BMC including a couple of I2C devices as found on real HW. Signed-off-by: Cédric Le Goater <clg@kaod.org> --- hw/arm/aspeed.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+)