On 7/26/24 01:52, Nicholas Piggin wrote:
> One of the functions of the ADU is indirect memory access engines that
> send and receive data via ADU registers.
>
> This implements the ADU LPC memory access functionality sufficiently
> for IBM proprietary firmware to access the UART and print characters
> to the serial port as it does on real hardware.
>
> This requires a linkage between adu and lpc, which allows adu to
> perform memory access in the lpc space.
>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> hw/ppc/pnv.c | 4 ++
> hw/ppc/pnv_adu.c | 95 ++++++++++++++++++++++++++++++++++++++++
> hw/ppc/pnv_lpc.c | 12 ++---
> include/hw/ppc/pnv_adu.h | 7 +++
> include/hw/ppc/pnv_lpc.h | 5 +++
> 5 files changed, 117 insertions(+), 6 deletions(-)
>
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 689197cbb7..91ff1be21f 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -1663,6 +1663,8 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
> }
>
> /* ADU */
> + object_property_set_link(OBJECT(&chip9->adu), "lpc", OBJECT(&chip9->lpc),
> + &error_abort);
> if (!qdev_realize(DEVICE(&chip9->adu), NULL, errp)) {
> return;
> }
> @@ -1929,6 +1931,8 @@ static void pnv_chip_power10_realize(DeviceState *dev, Error **errp)
> }
>
> /* ADU */
> + object_property_set_link(OBJECT(&chip10->adu), "lpc", OBJECT(&chip10->lpc),
> + &error_abort);
> if (!qdev_realize(DEVICE(&chip10->adu), NULL, errp)) {
> return;
> }
> diff --git a/hw/ppc/pnv_adu.c b/hw/ppc/pnv_adu.c
> index 8279bc8b26..81b7d6e526 100644
> --- a/hw/ppc/pnv_adu.c
> +++ b/hw/ppc/pnv_adu.c
> @@ -21,11 +21,18 @@
> #include "hw/ppc/pnv.h"
> #include "hw/ppc/pnv_adu.h"
> #include "hw/ppc/pnv_chip.h"
> +#include "hw/ppc/pnv_lpc.h"
> #include "hw/ppc/pnv_xscom.h"
> #include "trace.h"
>
> +#define ADU_LPC_BASE_REG 0x40
> +#define ADU_LPC_CMD_REG 0x41
> +#define ADU_LPC_DATA_REG 0x42
> +#define ADU_LPC_STATUS_REG 0x43
> +
> static uint64_t pnv_adu_xscom_read(void *opaque, hwaddr addr, unsigned width)
> {
> + PnvADU *adu = PNV_ADU(opaque);
> uint32_t offset = addr >> 3;
> uint64_t val = 0;
>
> @@ -34,6 +41,24 @@ static uint64_t pnv_adu_xscom_read(void *opaque, hwaddr addr, unsigned width)
> case 0x12: /* log register */
> case 0x13: /* error register */
> break;
> + case ADU_LPC_BASE_REG:
> + /*
> + * LPC Address Map in Pervasive ADU Workbook
> + *
> + * return PNV10_LPCM_BASE(chip) & PPC_BITMASK(8, 31);
> + * XXX: implement as class property, or get from LPC?
> + */
> + qemu_log_mask(LOG_UNIMP, "ADU: LPC_BASE_REG is not implemented\n");
> + break;
> + case ADU_LPC_CMD_REG:
> + val = adu->lpc_cmd_reg;
> + break;
> + case ADU_LPC_DATA_REG:
> + val = adu->lpc_data_reg;
> + break;
> + case ADU_LPC_STATUS_REG:
> + val = PPC_BIT(0); /* ack / done */
> + break;
>
> default:
> qemu_log_mask(LOG_UNIMP, "ADU Unimplemented read register: Ox%08x\n",
> @@ -45,9 +70,30 @@ static uint64_t pnv_adu_xscom_read(void *opaque, hwaddr addr, unsigned width)
> return val;
> }
>
> +static bool lpc_cmd_read(PnvADU *adu)
> +{
> + return !!(adu->lpc_cmd_reg & PPC_BIT(0));
> +}
> +
> +static bool lpc_cmd_write(PnvADU *adu)
> +{
> + return !lpc_cmd_read(adu);
> +}
> +
> +static uint32_t lpc_cmd_addr(PnvADU *adu)
> +{
> + return (adu->lpc_cmd_reg & PPC_BITMASK(32, 63)) >> PPC_BIT_NR(63);
> +}
> +
> +static uint32_t lpc_cmd_size(PnvADU *adu)
> +{
> + return (adu->lpc_cmd_reg & PPC_BITMASK(5, 11)) >> PPC_BIT_NR(11);
> +}
> +
> static void pnv_adu_xscom_write(void *opaque, hwaddr addr, uint64_t val,
> unsigned width)
> {
> + PnvADU *adu = PNV_ADU(opaque);
> uint32_t offset = addr >> 3;
>
> trace_pnv_adu_xscom_write(addr, val);
> @@ -58,6 +104,47 @@ static void pnv_adu_xscom_write(void *opaque, hwaddr addr, uint64_t val,
> case 0x13: /* error register */
> break;
>
> + case ADU_LPC_BASE_REG:
> + qemu_log_mask(LOG_UNIMP,
> + "ADU: Changing LPC_BASE_REG is not implemented\n");
> + break;
> +
> + case ADU_LPC_CMD_REG:
> + adu->lpc_cmd_reg = val;
> + if (lpc_cmd_read(adu)) {
> + uint32_t lpc_addr = lpc_cmd_addr(adu);
> + uint32_t lpc_size = lpc_cmd_size(adu);
> + uint64_t data = 0;
> +
> + pnv_lpc_opb_read(adu->lpc, lpc_addr, (void *)&data, lpc_size);
Coverity reports an issue :
>>> CID 1558830: (OVERRUN)
>>> Overrunning buffer pointed to by "(void *)&data" of 8 bytes by passing it to a function which accesses it at byte offset 126 using argument "lpc_size" (which evaluates to 127).
I think we should change the type of variable 'data' to allow reading
a maximum of 128 bytes to cover all possible LPC access. Also, possible
sizes should be limited to power of 2.
Thanks,
C.
> +
> + /*
> + * ADU access is performed within 8-byte aligned sectors. Smaller
> + * access sizes don't get formatted to the least significant byte,
> + * but rather appear in the data reg at the same offset as the
> + * address in memory. This shifts them into that position.
> + */
> + adu->lpc_data_reg = be64_to_cpu(data) >> ((lpc_addr & 7) * 8);
> + }
> + break;
> +
> + case ADU_LPC_DATA_REG:
> + adu->lpc_data_reg = val;
> + if (lpc_cmd_write(adu)) {
> + uint32_t lpc_addr = lpc_cmd_addr(adu);
> + uint32_t lpc_size = lpc_cmd_size(adu);
> + uint64_t data;
> +
> + data = cpu_to_be64(val) >> ((lpc_addr & 7) * 8); /* See above */
> + pnv_lpc_opb_write(adu->lpc, lpc_addr, (void *)&data, lpc_size);
> + }
> + break;
> +
> + case ADU_LPC_STATUS_REG:
> + qemu_log_mask(LOG_UNIMP,
> + "ADU: Changing LPC_STATUS_REG is not implemented\n");
> + break;
> +
> default:
> qemu_log_mask(LOG_UNIMP, "ADU Unimplemented write register: Ox%08x\n",
> offset);
> @@ -78,18 +165,26 @@ static void pnv_adu_realize(DeviceState *dev, Error **errp)
> {
> PnvADU *adu = PNV_ADU(dev);
>
> + assert(adu->lpc);
> +
> /* XScom regions for ADU registers */
> pnv_xscom_region_init(&adu->xscom_regs, OBJECT(dev),
> &pnv_adu_xscom_ops, adu, "xscom-adu",
> PNV9_XSCOM_ADU_SIZE);
> }
>
> +static Property pnv_adu_properties[] = {
> + DEFINE_PROP_LINK("lpc", PnvADU, lpc, TYPE_PNV_LPC, PnvLpcController *),
> + DEFINE_PROP_END_OF_LIST(),
> +};
> +
> static void pnv_adu_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
>
> dc->realize = pnv_adu_realize;
> dc->desc = "PowerNV ADU";
> + device_class_set_props(dc, pnv_adu_properties);
> dc->user_creatable = false;
> }
>
> diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
> index 0f14e180af..f8aad955b5 100644
> --- a/hw/ppc/pnv_lpc.c
> +++ b/hw/ppc/pnv_lpc.c
> @@ -236,16 +236,16 @@ int pnv_dt_lpc(PnvChip *chip, void *fdt, int root_offset, uint64_t lpcm_addr,
> * TODO: rework to use address_space_stq() and address_space_ldq()
> * instead.
> */
> -static bool opb_read(PnvLpcController *lpc, uint32_t addr, uint8_t *data,
> - int sz)
> +bool pnv_lpc_opb_read(PnvLpcController *lpc, uint32_t addr,
> + uint8_t *data, int sz)
> {
> /* XXX Handle access size limits and FW read caching here */
> return !address_space_read(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED,
> data, sz);
> }
>
> -static bool opb_write(PnvLpcController *lpc, uint32_t addr, uint8_t *data,
> - int sz)
> +bool pnv_lpc_opb_write(PnvLpcController *lpc, uint32_t addr,
> + uint8_t *data, int sz)
> {
> /* XXX Handle access size limits here */
> return !address_space_write(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED,
> @@ -277,7 +277,7 @@ static void pnv_lpc_do_eccb(PnvLpcController *lpc, uint64_t cmd)
> }
>
> if (cmd & ECCB_CTL_READ) {
> - success = opb_read(lpc, opb_addr, data, sz);
> + success = pnv_lpc_opb_read(lpc, opb_addr, data, sz);
> if (success) {
> lpc->eccb_stat_reg = ECCB_STAT_OP_DONE |
> (((uint64_t)data[0]) << 24 |
> @@ -294,7 +294,7 @@ static void pnv_lpc_do_eccb(PnvLpcController *lpc, uint64_t cmd)
> data[2] = lpc->eccb_data_reg >> 8;
> data[3] = lpc->eccb_data_reg;
>
> - success = opb_write(lpc, opb_addr, data, sz);
> + success = pnv_lpc_opb_write(lpc, opb_addr, data, sz);
> lpc->eccb_stat_reg = ECCB_STAT_OP_DONE;
> }
> /* XXX Which error bit (if any) to signal OPB error ? */
> diff --git a/include/hw/ppc/pnv_adu.h b/include/hw/ppc/pnv_adu.h
> index b5f308627b..f9dbd8c8b3 100644
> --- a/include/hw/ppc/pnv_adu.h
> +++ b/include/hw/ppc/pnv_adu.h
> @@ -10,6 +10,7 @@
> #define PPC_PNV_ADU_H
>
> #include "hw/ppc/pnv.h"
> +#include "hw/ppc/pnv_lpc.h"
> #include "hw/qdev-core.h"
>
> #define TYPE_PNV_ADU "pnv-adu"
> @@ -19,6 +20,12 @@ OBJECT_DECLARE_TYPE(PnvADU, PnvADUClass, PNV_ADU)
> struct PnvADU {
> DeviceState xd;
>
> + /* LPCMC (LPC Master Controller) access engine */
> + PnvLpcController *lpc;
> + uint64_t lpc_base_reg;
> + uint64_t lpc_cmd_reg;
> + uint64_t lpc_data_reg;
> +
> MemoryRegion xscom_regs;
> };
>
> diff --git a/include/hw/ppc/pnv_lpc.h b/include/hw/ppc/pnv_lpc.h
> index e0fd5e4130..174add4c53 100644
> --- a/include/hw/ppc/pnv_lpc.h
> +++ b/include/hw/ppc/pnv_lpc.h
> @@ -109,6 +109,11 @@ struct PnvLpcClass {
> DeviceRealize parent_realize;
> };
>
> +bool pnv_lpc_opb_read(PnvLpcController *lpc, uint32_t addr,
> + uint8_t *data, int sz);
> +bool pnv_lpc_opb_write(PnvLpcController *lpc, uint32_t addr,
> + uint8_t *data, int sz);
> +
> ISABus *pnv_lpc_isa_create(PnvLpcController *lpc, bool use_cpld, Error **errp);
> int pnv_dt_lpc(PnvChip *chip, void *fdt, int root_offset,
> uint64_t lpcm_addr, uint64_t lpcm_size);