Re: [Qemu-devel] [PATCH 18/27] ppc/pnv: add a LPC Controller model for POWER9

2019-03-07 Thread Cédric Le Goater
On 3/8/19 1:19 AM, David Gibson wrote:
> On Thu, Mar 07, 2019 at 08:07:41AM +0100, Cédric Le Goater wrote:
>> On 3/7/19 5:18 AM, David Gibson wrote:
>>> On Wed, Mar 06, 2019 at 09:50:23AM +0100, Cédric Le Goater wrote:
> [snip]
 +static uint64_t pnv_lpc_mmio_read(void *opaque, hwaddr addr, unsigned 
 size)
 +{
 +PnvLpcController *lpc = PNV_LPC(opaque);
 +uint64_t val = 0;
 +uint32_t opb_addr = addr & ECCB_CTL_ADDR_MASK;
 +MemTxResult result;
 +
 +switch (size) {
 +case 4:
 +val = address_space_ldl(>opb_as, opb_addr, 
 MEMTXATTRS_UNSPECIFIED,
 +);
 +break;
 +case 1:
 +val = address_space_ldub(>opb_as, opb_addr, 
 MEMTXATTRS_UNSPECIFIED,
 + );
 +break;
 +default:
 +qemu_log_mask(LOG_GUEST_ERROR, "OPB read failed at @0x%"
 +  HWADDR_PRIx " invalid size %d\n", addr, size);
 +return 0;
 +}
 +
 +if (result != MEMTX_OK) {
 +qemu_log_mask(LOG_GUEST_ERROR, "OPB read failed at @0x%"
 +  HWADDR_PRIx "\n", addr);
 +}
 +
 +return val;
>>>
>>> Couldn't you just map the relevant portion of the OPB AS into the MMIO
>>> AS, rather than having to forward the IOs with explicit read/write
>>> functions?
>>
>> The underlying memory regions (ISA space, LPC HC, OPB regs) are the
>> same on POWER8. So this is one way to share the overall initialization. 
> 
> I don't understand how that relates to my question.  If anything that
> sounds like it makes even more sense to map the common MR with the
> actual registers into the MMIO AS as a subregion, rather than having a
> redirect via the OPB AS.

ok. I will try that.

Thanks,

C.   

> 
>> What I would have liked to do is to simplified the ECCB interface 
>> (see pnv_lpc_do_eccb()).
>>
>> Thanks,
>>
>> C. 
>>
>>
 +}
 +
 +static void pnv_lpc_mmio_write(void *opaque, hwaddr addr,
 +uint64_t val, unsigned size)
 +{
 +PnvLpcController *lpc = PNV_LPC(opaque);
 +uint32_t opb_addr = addr & ECCB_CTL_ADDR_MASK;
 +MemTxResult result;
 +
 +switch (size) {
 +case 4:
 +address_space_stl(>opb_as, opb_addr, val, 
 MEMTXATTRS_UNSPECIFIED,
 +  );
 + break;
 +case 1:
 +address_space_stb(>opb_as, opb_addr, val, 
 MEMTXATTRS_UNSPECIFIED,
 +  );
 +break;
 +default:
 +qemu_log_mask(LOG_GUEST_ERROR, "OPB write failed at @0x%"
 +  HWADDR_PRIx " invalid size %d\n", addr, size);
 +return;
 +}
 +
 +if (result != MEMTX_OK) {
 +qemu_log_mask(LOG_GUEST_ERROR, "OPB write failed at @0x%"
 +  HWADDR_PRIx "\n", addr);
 +}
 +}
 +
 +static const MemoryRegionOps pnv_lpc_mmio_ops = {
 +.read = pnv_lpc_mmio_read,
 +.write = pnv_lpc_mmio_write,
 +.impl = {
 +.min_access_size = 1,
 +.max_access_size = 4,
 +},
 +.endianness = DEVICE_BIG_ENDIAN,
 +};
 +
  static void pnv_lpc_eval_irqs(PnvLpcController *lpc)
  {
  bool lpc_to_opb_irq = false;
 @@ -465,6 +627,43 @@ static const TypeInfo pnv_lpc_power8_info = {
  }
  };
  
 +static void pnv_lpc_power9_realize(DeviceState *dev, Error **errp)
 +{
 +PnvLpcController *lpc = PNV_LPC(dev);
 +PnvLpcClass *plc = PNV_LPC_GET_CLASS(dev);
 +Error *local_err = NULL;
 +
 +plc->parent_realize(dev, _err);
 +if (local_err) {
 +error_propagate(errp, local_err);
 +return;
 +}
 +
 +/* P9 uses a MMIO region */
 +memory_region_init_io(>xscom_regs, OBJECT(lpc), 
 _lpc_mmio_ops,
 +  lpc, "lpcm", PNV9_LPCM_SIZE);
 +}
 +
 +static void pnv_lpc_power9_class_init(ObjectClass *klass, void *data)
 +{
 +DeviceClass *dc = DEVICE_CLASS(klass);
 +PnvLpcClass *plc = PNV_LPC_CLASS(klass);
 +
 +dc->desc = "PowerNV LPC Controller POWER9";
 +
 +plc->psi_irq = PSIHB9_IRQ_LPCHC;
 +
 +device_class_set_parent_realize(dc, pnv_lpc_power9_realize,
 +>parent_realize);
 +}
 +
 +static const TypeInfo pnv_lpc_power9_info = {
 +.name  = TYPE_PNV9_LPC,
 +.parent= TYPE_PNV_LPC,
 +.instance_size = sizeof(PnvLpcController),
 +.class_init= pnv_lpc_power9_class_init,
 +};
 +
  static void pnv_lpc_realize(DeviceState *dev, Error **errp)
  {
  PnvLpcController *lpc = PNV_LPC(dev);
 @@ -540,6 +739,7 @@ static void 

Re: [Qemu-devel] [PATCH 18/27] ppc/pnv: add a LPC Controller model for POWER9

2019-03-07 Thread David Gibson
On Thu, Mar 07, 2019 at 08:07:41AM +0100, Cédric Le Goater wrote:
> On 3/7/19 5:18 AM, David Gibson wrote:
> > On Wed, Mar 06, 2019 at 09:50:23AM +0100, Cédric Le Goater wrote:
[snip]
> >> +static uint64_t pnv_lpc_mmio_read(void *opaque, hwaddr addr, unsigned 
> >> size)
> >> +{
> >> +PnvLpcController *lpc = PNV_LPC(opaque);
> >> +uint64_t val = 0;
> >> +uint32_t opb_addr = addr & ECCB_CTL_ADDR_MASK;
> >> +MemTxResult result;
> >> +
> >> +switch (size) {
> >> +case 4:
> >> +val = address_space_ldl(>opb_as, opb_addr, 
> >> MEMTXATTRS_UNSPECIFIED,
> >> +);
> >> +break;
> >> +case 1:
> >> +val = address_space_ldub(>opb_as, opb_addr, 
> >> MEMTXATTRS_UNSPECIFIED,
> >> + );
> >> +break;
> >> +default:
> >> +qemu_log_mask(LOG_GUEST_ERROR, "OPB read failed at @0x%"
> >> +  HWADDR_PRIx " invalid size %d\n", addr, size);
> >> +return 0;
> >> +}
> >> +
> >> +if (result != MEMTX_OK) {
> >> +qemu_log_mask(LOG_GUEST_ERROR, "OPB read failed at @0x%"
> >> +  HWADDR_PRIx "\n", addr);
> >> +}
> >> +
> >> +return val;
> > 
> > Couldn't you just map the relevant portion of the OPB AS into the MMIO
> > AS, rather than having to forward the IOs with explicit read/write
> > functions?
> 
> The underlying memory regions (ISA space, LPC HC, OPB regs) are the
> same on POWER8. So this is one way to share the overall initialization. 

I don't understand how that relates to my question.  If anything that
sounds like it makes even more sense to map the common MR with the
actual registers into the MMIO AS as a subregion, rather than having a
redirect via the OPB AS.

> What I would have liked to do is to simplified the ECCB interface 
> (see pnv_lpc_do_eccb()).
> 
> Thanks,
> 
> C. 
> 
> 
> >> +}
> >> +
> >> +static void pnv_lpc_mmio_write(void *opaque, hwaddr addr,
> >> +uint64_t val, unsigned size)
> >> +{
> >> +PnvLpcController *lpc = PNV_LPC(opaque);
> >> +uint32_t opb_addr = addr & ECCB_CTL_ADDR_MASK;
> >> +MemTxResult result;
> >> +
> >> +switch (size) {
> >> +case 4:
> >> +address_space_stl(>opb_as, opb_addr, val, 
> >> MEMTXATTRS_UNSPECIFIED,
> >> +  );
> >> + break;
> >> +case 1:
> >> +address_space_stb(>opb_as, opb_addr, val, 
> >> MEMTXATTRS_UNSPECIFIED,
> >> +  );
> >> +break;
> >> +default:
> >> +qemu_log_mask(LOG_GUEST_ERROR, "OPB write failed at @0x%"
> >> +  HWADDR_PRIx " invalid size %d\n", addr, size);
> >> +return;
> >> +}
> >> +
> >> +if (result != MEMTX_OK) {
> >> +qemu_log_mask(LOG_GUEST_ERROR, "OPB write failed at @0x%"
> >> +  HWADDR_PRIx "\n", addr);
> >> +}
> >> +}
> >> +
> >> +static const MemoryRegionOps pnv_lpc_mmio_ops = {
> >> +.read = pnv_lpc_mmio_read,
> >> +.write = pnv_lpc_mmio_write,
> >> +.impl = {
> >> +.min_access_size = 1,
> >> +.max_access_size = 4,
> >> +},
> >> +.endianness = DEVICE_BIG_ENDIAN,
> >> +};
> >> +
> >>  static void pnv_lpc_eval_irqs(PnvLpcController *lpc)
> >>  {
> >>  bool lpc_to_opb_irq = false;
> >> @@ -465,6 +627,43 @@ static const TypeInfo pnv_lpc_power8_info = {
> >>  }
> >>  };
> >>  
> >> +static void pnv_lpc_power9_realize(DeviceState *dev, Error **errp)
> >> +{
> >> +PnvLpcController *lpc = PNV_LPC(dev);
> >> +PnvLpcClass *plc = PNV_LPC_GET_CLASS(dev);
> >> +Error *local_err = NULL;
> >> +
> >> +plc->parent_realize(dev, _err);
> >> +if (local_err) {
> >> +error_propagate(errp, local_err);
> >> +return;
> >> +}
> >> +
> >> +/* P9 uses a MMIO region */
> >> +memory_region_init_io(>xscom_regs, OBJECT(lpc), 
> >> _lpc_mmio_ops,
> >> +  lpc, "lpcm", PNV9_LPCM_SIZE);
> >> +}
> >> +
> >> +static void pnv_lpc_power9_class_init(ObjectClass *klass, void *data)
> >> +{
> >> +DeviceClass *dc = DEVICE_CLASS(klass);
> >> +PnvLpcClass *plc = PNV_LPC_CLASS(klass);
> >> +
> >> +dc->desc = "PowerNV LPC Controller POWER9";
> >> +
> >> +plc->psi_irq = PSIHB9_IRQ_LPCHC;
> >> +
> >> +device_class_set_parent_realize(dc, pnv_lpc_power9_realize,
> >> +>parent_realize);
> >> +}
> >> +
> >> +static const TypeInfo pnv_lpc_power9_info = {
> >> +.name  = TYPE_PNV9_LPC,
> >> +.parent= TYPE_PNV_LPC,
> >> +.instance_size = sizeof(PnvLpcController),
> >> +.class_init= pnv_lpc_power9_class_init,
> >> +};
> >> +
> >>  static void pnv_lpc_realize(DeviceState *dev, Error **errp)
> >>  {
> >>  PnvLpcController *lpc = PNV_LPC(dev);
> >> @@ -540,6 +739,7 @@ static void pnv_lpc_register_types(void)
> >>  {
> >>  type_register_static(_lpc_info);
> >>  

Re: [Qemu-devel] [PATCH 18/27] ppc/pnv: add a LPC Controller model for POWER9

2019-03-06 Thread Cédric Le Goater
On 3/7/19 5:18 AM, David Gibson wrote:
> On Wed, Mar 06, 2019 at 09:50:23AM +0100, Cédric Le Goater wrote:
>> The LPC Controller on POWER9 is very similar to the one found on
>> POWER8 but accesses are now done via on MMIOs, without the XSCOM and
>> ECCB logic. The device tree is populated differently so we add a
>> specific POWER9 routine for the purpose.
>>
>> SerIRQ routing is yet to be done.
>>
>> Signed-off-by: Cédric Le Goater 
>> ---
>>  include/hw/ppc/pnv.h |   4 +
>>  include/hw/ppc/pnv_lpc.h |  10 ++
>>  hw/ppc/pnv.c |  29 +-
>>  hw/ppc/pnv_lpc.c | 200 +++
>>  4 files changed, 240 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
>> index 57d0337219be..2d68aabc212f 100644
>> --- a/include/hw/ppc/pnv.h
>> +++ b/include/hw/ppc/pnv.h
>> @@ -85,6 +85,7 @@ typedef struct Pnv9Chip {
>>  /*< public >*/
>>  PnvXive  xive;
>>  PnvPsi   psi;
>> +PnvLpcController lpc;
>>  } Pnv9Chip;
>>  
>>  typedef struct PnvChipClass {
>> @@ -232,6 +233,9 @@ void pnv_bmc_powerdown(IPMIBmc *bmc);
>>  #define PNV9_XIVE_PC_SIZE0x0010ull
>>  #define PNV9_XIVE_PC_BASE(chip)  PNV9_CHIP_BASE(chip, 
>> 0x00060180ull)
>>  
>> +#define PNV9_LPCM_SIZE   0x0001ull
>> +#define PNV9_LPCM_BASE(chip) PNV9_CHIP_BASE(chip, 
>> 0x00060300ull)
>> +
>>  #define PNV9_PSIHB_SIZE  0x0010ull
>>  #define PNV9_PSIHB_BASE(chip)PNV9_CHIP_BASE(chip, 
>> 0x000603020300ull)
>>  
>> diff --git a/include/hw/ppc/pnv_lpc.h b/include/hw/ppc/pnv_lpc.h
>> index 242baecdcb93..24fe23f0f63b 100644
>> --- a/include/hw/ppc/pnv_lpc.h
>> +++ b/include/hw/ppc/pnv_lpc.h
>> @@ -28,6 +28,10 @@
>>  #define PNV8_LPC(obj) \
>>  OBJECT_CHECK(PnvLpc, (obj), TYPE_PNV8_LPC)
>>  
>> +#define TYPE_PNV9_LPC TYPE_PNV_LPC "-POWER9"
>> +#define PNV9_LPC(obj) \
>> +OBJECT_CHECK(PnvLpc, (obj), TYPE_PNV9_LPC)
>> +
>>  typedef struct PnvLpcController {
>>  DeviceState parent;
>>  
>> @@ -86,6 +90,12 @@ typedef struct PnvLpcClass {
>>  DeviceRealize parent_realize;
>>  } PnvLpcClass;
>>  
>> +/*
>> + * Old compilers error on typdef forward declarations. Keep them happy.
>> + */
>> +struct PnvChip;
>> +
>>  ISABus *pnv_lpc_isa_create(PnvLpcController *lpc, bool use_cpld, Error 
>> **errp);
>> +int pnv_dt_lpc(struct PnvChip *chip, void *fdt, int root_offset);
>>  
>>  #endif /* _PPC_PNV_LPC_H */
>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
>> index 7176e1b68d0e..895be470af67 100644
>> --- a/hw/ppc/pnv.c
>> +++ b/hw/ppc/pnv.c
>> @@ -306,6 +306,8 @@ static void pnv_chip_power9_dt_populate(PnvChip *chip, 
>> void *fdt)
>>  if (chip->ram_size) {
>>  pnv_dt_memory(fdt, chip->chip_id, chip->ram_start, chip->ram_size);
>>  }
>> +
>> +pnv_dt_lpc(chip, fdt, 0);
>>  }
>>  
>>  static void pnv_dt_rtc(ISADevice *d, void *fdt, int lpc_off)
>> @@ -422,8 +424,14 @@ static int pnv_chip_isa_offset(PnvChip *chip, void *fdt)
>>  char *name;
>>  int offset;
>>  
>> -name = g_strdup_printf("/xscom@%" PRIx64 "/isa@%x",
>> -   (uint64_t) PNV_XSCOM_BASE(chip), 
>> PNV_XSCOM_LPC_BASE);
>> +if (pnv_chip_is_power9(chip)) {
> 
> Again explicit chip type checks aren't very pretty.  Better to
> redirect through a method.  Or, better, call into the common ISA DT
> code from chip specific code passing the offset, rather than calling
> out again to get it.

I will an ISA DT nodename under PnvChip. The name can be computed 
at realize time.
 
> 
>> +name = g_strdup_printf("/lpcm-opb@%" PRIx64 "/lpc@0",
>> +   (uint64_t) PNV9_LPCM_BASE(chip));
>> +} else {
>> +name = g_strdup_printf("/xscom@%" PRIx64 "/isa@%x",
>> +   (uint64_t) PNV_XSCOM_BASE(chip),
>> +   PNV_XSCOM_LPC_BASE);
>> +}
>>  offset = fdt_path_offset(fdt, name);
>>  g_free(name);
>>  return offset;
>> @@ -559,7 +567,8 @@ static ISABus *pnv_chip_power8nvl_isa_create(PnvChip 
>> *chip, Error **errp)
>>  
>>  static ISABus *pnv_chip_power9_isa_create(PnvChip *chip, Error **errp)
>>  {
>> -return NULL;
>> +Pnv9Chip *chip9 = PNV9_CHIP(chip);
>> +return pnv_lpc_isa_create(>lpc, false, errp);
>>  }
>>  
>>  static ISABus *pnv_isa_create(PnvChip *chip, Error **errp)
>> @@ -954,6 +963,11 @@ static void pnv_chip_power9_instance_init(Object *obj)
>>  TYPE_PNV9_PSI, _abort, NULL);
>>  object_property_add_const_link(OBJECT(>psi), "chip", obj,
>> _abort);
>> +
>> +object_initialize_child(obj, "lpc",  >lpc, sizeof(chip9->lpc),
>> +TYPE_PNV9_LPC, _abort, NULL);
>> +object_property_add_const_link(OBJECT(>lpc), "psi",
>> +   OBJECT(>psi), _abort);
>>  }
>>  
>>  static void 

Re: [Qemu-devel] [PATCH 18/27] ppc/pnv: add a LPC Controller model for POWER9

2019-03-06 Thread David Gibson
On Wed, Mar 06, 2019 at 09:50:23AM +0100, Cédric Le Goater wrote:
> The LPC Controller on POWER9 is very similar to the one found on
> POWER8 but accesses are now done via on MMIOs, without the XSCOM and
> ECCB logic. The device tree is populated differently so we add a
> specific POWER9 routine for the purpose.
> 
> SerIRQ routing is yet to be done.
> 
> Signed-off-by: Cédric Le Goater 
> ---
>  include/hw/ppc/pnv.h |   4 +
>  include/hw/ppc/pnv_lpc.h |  10 ++
>  hw/ppc/pnv.c |  29 +-
>  hw/ppc/pnv_lpc.c | 200 +++
>  4 files changed, 240 insertions(+), 3 deletions(-)
> 
> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> index 57d0337219be..2d68aabc212f 100644
> --- a/include/hw/ppc/pnv.h
> +++ b/include/hw/ppc/pnv.h
> @@ -85,6 +85,7 @@ typedef struct Pnv9Chip {
>  /*< public >*/
>  PnvXive  xive;
>  PnvPsi   psi;
> +PnvLpcController lpc;
>  } Pnv9Chip;
>  
>  typedef struct PnvChipClass {
> @@ -232,6 +233,9 @@ void pnv_bmc_powerdown(IPMIBmc *bmc);
>  #define PNV9_XIVE_PC_SIZE0x0010ull
>  #define PNV9_XIVE_PC_BASE(chip)  PNV9_CHIP_BASE(chip, 
> 0x00060180ull)
>  
> +#define PNV9_LPCM_SIZE   0x0001ull
> +#define PNV9_LPCM_BASE(chip) PNV9_CHIP_BASE(chip, 
> 0x00060300ull)
> +
>  #define PNV9_PSIHB_SIZE  0x0010ull
>  #define PNV9_PSIHB_BASE(chip)PNV9_CHIP_BASE(chip, 
> 0x000603020300ull)
>  
> diff --git a/include/hw/ppc/pnv_lpc.h b/include/hw/ppc/pnv_lpc.h
> index 242baecdcb93..24fe23f0f63b 100644
> --- a/include/hw/ppc/pnv_lpc.h
> +++ b/include/hw/ppc/pnv_lpc.h
> @@ -28,6 +28,10 @@
>  #define PNV8_LPC(obj) \
>  OBJECT_CHECK(PnvLpc, (obj), TYPE_PNV8_LPC)
>  
> +#define TYPE_PNV9_LPC TYPE_PNV_LPC "-POWER9"
> +#define PNV9_LPC(obj) \
> +OBJECT_CHECK(PnvLpc, (obj), TYPE_PNV9_LPC)
> +
>  typedef struct PnvLpcController {
>  DeviceState parent;
>  
> @@ -86,6 +90,12 @@ typedef struct PnvLpcClass {
>  DeviceRealize parent_realize;
>  } PnvLpcClass;
>  
> +/*
> + * Old compilers error on typdef forward declarations. Keep them happy.
> + */
> +struct PnvChip;
> +
>  ISABus *pnv_lpc_isa_create(PnvLpcController *lpc, bool use_cpld, Error 
> **errp);
> +int pnv_dt_lpc(struct PnvChip *chip, void *fdt, int root_offset);
>  
>  #endif /* _PPC_PNV_LPC_H */
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 7176e1b68d0e..895be470af67 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -306,6 +306,8 @@ static void pnv_chip_power9_dt_populate(PnvChip *chip, 
> void *fdt)
>  if (chip->ram_size) {
>  pnv_dt_memory(fdt, chip->chip_id, chip->ram_start, chip->ram_size);
>  }
> +
> +pnv_dt_lpc(chip, fdt, 0);
>  }
>  
>  static void pnv_dt_rtc(ISADevice *d, void *fdt, int lpc_off)
> @@ -422,8 +424,14 @@ static int pnv_chip_isa_offset(PnvChip *chip, void *fdt)
>  char *name;
>  int offset;
>  
> -name = g_strdup_printf("/xscom@%" PRIx64 "/isa@%x",
> -   (uint64_t) PNV_XSCOM_BASE(chip), 
> PNV_XSCOM_LPC_BASE);
> +if (pnv_chip_is_power9(chip)) {

Again explicit chip type checks aren't very pretty.  Better to
redirect through a method.  Or, better, call into the common ISA DT
code from chip specific code passing the offset, rather than calling
out again to get it.

> +name = g_strdup_printf("/lpcm-opb@%" PRIx64 "/lpc@0",
> +   (uint64_t) PNV9_LPCM_BASE(chip));
> +} else {
> +name = g_strdup_printf("/xscom@%" PRIx64 "/isa@%x",
> +   (uint64_t) PNV_XSCOM_BASE(chip),
> +   PNV_XSCOM_LPC_BASE);
> +}
>  offset = fdt_path_offset(fdt, name);
>  g_free(name);
>  return offset;
> @@ -559,7 +567,8 @@ static ISABus *pnv_chip_power8nvl_isa_create(PnvChip 
> *chip, Error **errp)
>  
>  static ISABus *pnv_chip_power9_isa_create(PnvChip *chip, Error **errp)
>  {
> -return NULL;
> +Pnv9Chip *chip9 = PNV9_CHIP(chip);
> +return pnv_lpc_isa_create(>lpc, false, errp);
>  }
>  
>  static ISABus *pnv_isa_create(PnvChip *chip, Error **errp)
> @@ -954,6 +963,11 @@ static void pnv_chip_power9_instance_init(Object *obj)
>  TYPE_PNV9_PSI, _abort, NULL);
>  object_property_add_const_link(OBJECT(>psi), "chip", obj,
> _abort);
> +
> +object_initialize_child(obj, "lpc",  >lpc, sizeof(chip9->lpc),
> +TYPE_PNV9_LPC, _abort, NULL);
> +object_property_add_const_link(OBJECT(>lpc), "psi",
> +   OBJECT(>psi), _abort);
>  }
>  
>  static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
> @@ -997,6 +1011,15 @@ static void pnv_chip_power9_realize(DeviceState *dev, 
> Error **errp)
>  }
>  pnv_xscom_add_subregion(chip, PNV9_XSCOM_PSIHB_BASE,
>  >psi.xscom_regs);
> +
> +/* LPC */
> 

[Qemu-devel] [PATCH 18/27] ppc/pnv: add a LPC Controller model for POWER9

2019-03-06 Thread Cédric Le Goater
The LPC Controller on POWER9 is very similar to the one found on
POWER8 but accesses are now done via on MMIOs, without the XSCOM and
ECCB logic. The device tree is populated differently so we add a
specific POWER9 routine for the purpose.

SerIRQ routing is yet to be done.

Signed-off-by: Cédric Le Goater 
---
 include/hw/ppc/pnv.h |   4 +
 include/hw/ppc/pnv_lpc.h |  10 ++
 hw/ppc/pnv.c |  29 +-
 hw/ppc/pnv_lpc.c | 200 +++
 4 files changed, 240 insertions(+), 3 deletions(-)

diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
index 57d0337219be..2d68aabc212f 100644
--- a/include/hw/ppc/pnv.h
+++ b/include/hw/ppc/pnv.h
@@ -85,6 +85,7 @@ typedef struct Pnv9Chip {
 /*< public >*/
 PnvXive  xive;
 PnvPsi   psi;
+PnvLpcController lpc;
 } Pnv9Chip;
 
 typedef struct PnvChipClass {
@@ -232,6 +233,9 @@ void pnv_bmc_powerdown(IPMIBmc *bmc);
 #define PNV9_XIVE_PC_SIZE0x0010ull
 #define PNV9_XIVE_PC_BASE(chip)  PNV9_CHIP_BASE(chip, 
0x00060180ull)
 
+#define PNV9_LPCM_SIZE   0x0001ull
+#define PNV9_LPCM_BASE(chip) PNV9_CHIP_BASE(chip, 
0x00060300ull)
+
 #define PNV9_PSIHB_SIZE  0x0010ull
 #define PNV9_PSIHB_BASE(chip)PNV9_CHIP_BASE(chip, 
0x000603020300ull)
 
diff --git a/include/hw/ppc/pnv_lpc.h b/include/hw/ppc/pnv_lpc.h
index 242baecdcb93..24fe23f0f63b 100644
--- a/include/hw/ppc/pnv_lpc.h
+++ b/include/hw/ppc/pnv_lpc.h
@@ -28,6 +28,10 @@
 #define PNV8_LPC(obj) \
 OBJECT_CHECK(PnvLpc, (obj), TYPE_PNV8_LPC)
 
+#define TYPE_PNV9_LPC TYPE_PNV_LPC "-POWER9"
+#define PNV9_LPC(obj) \
+OBJECT_CHECK(PnvLpc, (obj), TYPE_PNV9_LPC)
+
 typedef struct PnvLpcController {
 DeviceState parent;
 
@@ -86,6 +90,12 @@ typedef struct PnvLpcClass {
 DeviceRealize parent_realize;
 } PnvLpcClass;
 
+/*
+ * Old compilers error on typdef forward declarations. Keep them happy.
+ */
+struct PnvChip;
+
 ISABus *pnv_lpc_isa_create(PnvLpcController *lpc, bool use_cpld, Error **errp);
+int pnv_dt_lpc(struct PnvChip *chip, void *fdt, int root_offset);
 
 #endif /* _PPC_PNV_LPC_H */
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 7176e1b68d0e..895be470af67 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -306,6 +306,8 @@ static void pnv_chip_power9_dt_populate(PnvChip *chip, void 
*fdt)
 if (chip->ram_size) {
 pnv_dt_memory(fdt, chip->chip_id, chip->ram_start, chip->ram_size);
 }
+
+pnv_dt_lpc(chip, fdt, 0);
 }
 
 static void pnv_dt_rtc(ISADevice *d, void *fdt, int lpc_off)
@@ -422,8 +424,14 @@ static int pnv_chip_isa_offset(PnvChip *chip, void *fdt)
 char *name;
 int offset;
 
-name = g_strdup_printf("/xscom@%" PRIx64 "/isa@%x",
-   (uint64_t) PNV_XSCOM_BASE(chip), 
PNV_XSCOM_LPC_BASE);
+if (pnv_chip_is_power9(chip)) {
+name = g_strdup_printf("/lpcm-opb@%" PRIx64 "/lpc@0",
+   (uint64_t) PNV9_LPCM_BASE(chip));
+} else {
+name = g_strdup_printf("/xscom@%" PRIx64 "/isa@%x",
+   (uint64_t) PNV_XSCOM_BASE(chip),
+   PNV_XSCOM_LPC_BASE);
+}
 offset = fdt_path_offset(fdt, name);
 g_free(name);
 return offset;
@@ -559,7 +567,8 @@ static ISABus *pnv_chip_power8nvl_isa_create(PnvChip *chip, 
Error **errp)
 
 static ISABus *pnv_chip_power9_isa_create(PnvChip *chip, Error **errp)
 {
-return NULL;
+Pnv9Chip *chip9 = PNV9_CHIP(chip);
+return pnv_lpc_isa_create(>lpc, false, errp);
 }
 
 static ISABus *pnv_isa_create(PnvChip *chip, Error **errp)
@@ -954,6 +963,11 @@ static void pnv_chip_power9_instance_init(Object *obj)
 TYPE_PNV9_PSI, _abort, NULL);
 object_property_add_const_link(OBJECT(>psi), "chip", obj,
_abort);
+
+object_initialize_child(obj, "lpc",  >lpc, sizeof(chip9->lpc),
+TYPE_PNV9_LPC, _abort, NULL);
+object_property_add_const_link(OBJECT(>lpc), "psi",
+   OBJECT(>psi), _abort);
 }
 
 static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
@@ -997,6 +1011,15 @@ static void pnv_chip_power9_realize(DeviceState *dev, 
Error **errp)
 }
 pnv_xscom_add_subregion(chip, PNV9_XSCOM_PSIHB_BASE,
 >psi.xscom_regs);
+
+/* LPC */
+object_property_set_bool(OBJECT(>lpc), true, "realized", 
_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
+memory_region_add_subregion(get_system_memory(), PNV9_LPCM_BASE(chip),
+>lpc.xscom_regs);
 }
 
 static void pnv_chip_power9_class_init(ObjectClass *klass, void *data)
diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
index 3c509a30a0af..6df694e0abc1 100644
--- a/hw/ppc/pnv_lpc.c
+++ b/hw/ppc/pnv_lpc.c
@@ -118,6 +118,100 @@ static int