Re: [Qemu-devel] [PATCH v2 06/15] ppc/pnv: add a LPC Controller model for POWER9

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 4823 bytes --]

On Thu, Mar 07, 2019 at 11:35:39PM +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 <clg@kaod.org>

[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(&lpc->opb_as, opb_addr, MEMTXATTRS_UNSPECIFIED,
> +                                &result);

This extra level of indirection via the opb_as still seems very
dubious to me.  But I guess it's something we can fix later, so,
applied.


> +        break;
> +    case 1:
> +        val = address_space_ldub(&lpc->opb_as, opb_addr, MEMTXATTRS_UNSPECIFIED,
> +                                 &result);
> +        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;
> +}
> +
> +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(&lpc->opb_as, opb_addr, val, MEMTXATTRS_UNSPECIFIED,
> +                          &result);
> +         break;
> +    case 1:
> +        address_space_stb(&lpc->opb_as, opb_addr, val, MEMTXATTRS_UNSPECIFIED,
> +                          &result);
> +        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, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +
> +    /* P9 uses a MMIO region */
> +    memory_region_init_io(&lpc->xscom_regs, OBJECT(lpc), &pnv_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,
> +                                    &plc->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(&pnv_lpc_info);
>      type_register_static(&pnv_lpc_power8_info);
> +    type_register_static(&pnv_lpc_power9_info);
>  }
>  
>  type_init(pnv_lpc_register_types)

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]