Re: [PATCH 2/4] Add MEN Chameleon Bus via PCI carrier

[Alistair Francis]

On Wed, Mar 29, 2023 at 11:09 PM Johannes Thumshirn <jth@kernel.org> wrote:
>
> Add PCI based MEN Chameleon Bus carrier emulation.
>
> Signed-off-by: Johannes Thumshirn <jth@kernel.org>
> ---
>  hw/mcb/Kconfig     |   6 +
>  hw/mcb/mcb-pci.c   | 307 +++++++++++++++++++++++++++++++++++++++++++++
>  hw/mcb/meson.build |   1 +
>  3 files changed, 314 insertions(+)
>  create mode 100644 hw/mcb/mcb-pci.c
>
> diff --git a/hw/mcb/Kconfig b/hw/mcb/Kconfig
> index 36a7a583a8..7deb96c2fe 100644
> --- a/hw/mcb/Kconfig
> +++ b/hw/mcb/Kconfig
> @@ -1,2 +1,8 @@
>  config MCB
>      bool
> +
> +config MCB_PCI
> +    bool
> +    default y if PCI_DEVICES
> +    depends on PCI
> +    select MCB
> diff --git a/hw/mcb/mcb-pci.c b/hw/mcb/mcb-pci.c
> new file mode 100644
> index 0000000000..442e65e24c
> --- /dev/null
> +++ b/hw/mcb/mcb-pci.c
> @@ -0,0 +1,307 @@
> +/*
> + * QEMU MEN Chameleon Bus emulation
> + *
> + * Copyright (C) 2023 Johannes Thumshirn <jth@kernel.org>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "hw/mcb/mcb.h"
> +#include "hw/pci/pci.h"
> +#include "hw/pci/pci_device.h"
> +#include "hw/qdev-properties.h"
> +#include "migration/vmstate.h"
> +
> +/* #define DEBUG_MPCI 1 */
> +
> +#ifdef DEBUG_MPCI
> +#define DPRINTF(fmt, ...)                                               \
> +    do { fprintf(stderr, "mcb-pci: " fmt, ## __VA_ARGS__); } while (0)
> +#else
> +#define DPRINTF(fmt, ...) do { } while (0)
> +#endif

We don't use these #ifdefs for debug prints anymore in QEMU (at least
not in new code). It should be converted to the tracing architecture.
See https://qemu-project.gitlab.io/qemu/devel/tracing.html for user
documentation.

> +
> +typedef struct {
> +    uint8_t revision;
> +    char model;
> +    uint8_t minor;
> +    uint8_t bus_type;
> +    uint16_t magic;
> +    uint16_t reserved;
> +    /* This one has no '\0' at the end!!! */
> +    char filename[12];
> +} ChameleonFPGAHeader;
> +#define CHAMELEON_BUS_TYPE_WISHBONE 0
> +#define CHAMELEONV2_MAGIC 0xabce
> +
> +typedef struct {
> +    PCIDevice dev;
> +    MCBus bus;
> +    MemoryRegion ctbl;
> +    uint16_t status;
> +    uint8_t int_set;
> +    ChameleonFPGAHeader *header;
> +
> +    uint8_t minor;
> +    uint8_t rev;
> +    uint8_t model;
> +} MPCIState;
> +
> +#define TYPE_MCB_PCI "mcb-pci"
> +
> +#define MPCI(obj)                                       \
> +    OBJECT_CHECK(MPCIState, (obj), TYPE_MCB_PCI)
> +
> +#define CHAMELEON_TABLE_SIZE 0x200
> +#define N_MODULES 32
> +
> +#define PCI_VENDOR_ID_MEN 0x1a88
> +#define PCI_DEVICE_ID_MEN_MCBPCI 0x4d45
> +
> +static uint32_t read_header(MPCIState *s, hwaddr addr)
> +{
> +    uint32_t ret = 0;
> +    ChameleonFPGAHeader *header = s->header;
> +
> +    switch (addr >> 2) {
> +    case 0:
> +        ret |= header->revision;
> +        ret |= header->model << 8;
> +        ret |= header->minor << 16;
> +        ret |= header->bus_type << 24;
> +        break;
> +    case 1:
> +        ret |= header->magic;
> +        ret |= header->reserved << 16;
> +        break;
> +    case 2:
> +        memcpy(&ret, header->filename, sizeof(uint32_t));
> +        break;
> +    case 3:
> +        memcpy(&ret, header->filename + sizeof(uint32_t),
> +               sizeof(uint32_t));
> +        break;
> +    case 4:
> +        memcpy(&ret, header->filename + 2 * sizeof(uint32_t),
> +               sizeof(uint32_t));
> +    }
> +
> +    return ret;
> +}
> +
> +static uint32_t read_gdd(MCBDevice *mdev, int reg)
> +{
> +    ChameleonDeviceDescriptor *gdd;
> +    uint32_t ret = 0;
> +
> +    gdd = mdev->gdd;
> +
> +    switch (reg) {
> +    case 0:
> +        ret = gdd->reg1;
> +        break;
> +    case 1:
> +        ret = gdd->reg2;
> +        break;
> +    case 2:
> +        ret = gdd->offset;
> +        break;
> +    case 3:
> +        ret = gdd->size;
> +        break;
> +    }
> +
> +    return ret;
> +}
> +
> +static uint64_t mpci_chamtbl_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    MPCIState *s = opaque;
> +    MCBus *bus = &s->bus;
> +    MCBDevice *mdev;
> +    uint32_t ret = 0;
> +
> +    DPRINTF("Read from address 0x%lx size %d\n", addr, size);
> +
> +    if (addr < sizeof(ChameleonFPGAHeader)) {
> +        return le32_to_cpu(read_header(s, addr));

I always thought you don't need the le32_to_cpu() as the
DEVICE_LITTLE_ENDIAN should handle this.

I do see other PCI devices with it though, so maybe it's a PCI thing

> +    } else if (addr >= sizeof(ChameleonFPGAHeader) &&
> +               addr < CHAMELEON_TABLE_SIZE) {
> +        /* Handle read on chameleon table */
> +        BusChild *kid;
> +        DeviceState *qdev;
> +        int slot;
> +        int offset;
> +        int i;
> +
> +        offset = addr - sizeof(ChameleonFPGAHeader);
> +        slot = offset / sizeof(ChameleonDeviceDescriptor);
> +
> +        kid = QTAILQ_FIRST(&BUS(bus)->children);
> +        for (i = 0; i < slot; i++) {
> +            kid = QTAILQ_NEXT(kid, sibling);
> +            if (!kid) { /* Last element */
> +                DPRINTF("Last element: 0x%08x\n", ~0U);
> +                return ~0U;
> +            }
> +        }
> +        qdev = kid->child;
> +        mdev = MCB_DEVICE(qdev);
> +        offset -= slot * 16;
> +
> +        ret = read_gdd(mdev, offset / 4);
> +        return le32_to_cpu(ret);
> +    }
> +
> +    return ret;

ret isn't used here

> +}
> +
> +static void mpci_chamtbl_write(void *opaque, hwaddr addr, uint64_t val,
> +                               unsigned size)
> +{
> +
> +    if (addr < CHAMELEON_TABLE_SIZE)
> +        DPRINTF("Invalid write to 0x%x: 0x%x\n", (unsigned) addr,
> +                (unsigned) val);

This isn't Linux, brackets are required :)

You can run checkpatch to catch these types of style issues.

Alistair

> +
> +    return;
> +}
> +
> +static const MemoryRegionOps mpci_chamtbl_ops = {
> +    .read = mpci_chamtbl_read,
> +    .write = mpci_chamtbl_write,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +    .valid = {
> +        .min_access_size = 4,
> +        .max_access_size = 4
> +    },
> +    .impl = {
> +        .min_access_size = 4,
> +        .max_access_size = 4
> +    },
> +};
> +
> +static void mcb_pci_set_irq(void *opaque, int intno, int level)
> +{
> +    MCBDevice *mdev = opaque;
> +    MCBus *bus = MCB_BUS(qdev_get_parent_bus(DEVICE(mdev)));
> +    PCIDevice *pcidev = PCI_DEVICE(BUS(bus)->parent);
> +    MPCIState *dev = MPCI(pcidev);
> +
> +    if (level) {
> +        pci_set_irq(&dev->dev, !dev->int_set);
> +        pci_set_irq(&dev->dev,  dev->int_set);
> +    } else {
> +        uint16_t level_status = dev->status;
> +
> +        if (level_status && !dev->int_set) {
> +            pci_irq_assert(&dev->dev);
> +            dev->int_set = 1;
> +        } else if (!level_status && dev->int_set) {
> +            pci_irq_deassert(&dev->dev);
> +            dev->int_set = 0;
> +        }
> +    }
> +}
> +
> +static void mcb_pci_write_config(PCIDevice *pci_dev, uint32_t address,
> +                                 uint32_t val, int len)
> +{
> +    pci_default_write_config(pci_dev, address, val, len);
> +}
> +
> +static void mcb_pci_realize(PCIDevice *pci_dev, Error **errp)
> +{
> +    MPCIState *s = MPCI(pci_dev);
> +    uint8_t *pci_conf = s->dev.config;
> +    ChameleonFPGAHeader *header;
> +    MCBus *bus = &s->bus;
> +
> +    header = g_new0(ChameleonFPGAHeader, 1);
> +
> +    s->header = header;
> +
> +    header->revision = s->rev;
> +    header->model = (char) s->model;
> +    header->minor = s->minor;
> +    header->bus_type = CHAMELEON_BUS_TYPE_WISHBONE;
> +    header->magic = CHAMELEONV2_MAGIC;
> +    memcpy(&header->filename, "QEMU MCB PCI", 12);
> +
> +    pci_dev->config_write = mcb_pci_write_config;
> +    pci_set_byte(pci_conf + PCI_INTERRUPT_PIN, 0x01); /* Interrupt pin A */
> +    pci_conf[PCI_COMMAND] = PCI_COMMAND_MEMORY;
> +
> +    mcb_bus_init(bus, sizeof(MCBus), DEVICE(pci_dev), N_MODULES, 
> mcb_pci_set_irq);
> +
> +    memory_region_init(&bus->mmio_region, OBJECT(s), "mcb-pci.mmio",
> +                       2048 * 1024);
> +    memory_region_init_io(&s->ctbl, OBJECT(s), &mpci_chamtbl_ops,
> +                          s, "mpci_chamtbl_ops", CHAMELEON_TABLE_SIZE);
> +    memory_region_add_subregion(&bus->mmio_region, 0, &s->ctbl);
> +    pci_register_bar(&s->dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY,
> +                     &bus->mmio_region);
> +
> +}
> +
> +static void mcb_pci_unrealize(PCIDevice *pci_dev)
> +{
> +    MPCIState *s = MPCI(pci_dev);
> +
> +    g_free(s->header);
> +    s->header = NULL;
> +}
> +
> +static const VMStateDescription vmstate_mcb_pci = {
> +    .name = "mcb-pci",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_PCI_DEVICE(dev, MPCIState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static Property mcb_pci_props[] = {
> +    DEFINE_PROP_UINT8("revision", MPCIState, rev, 1),
> +    DEFINE_PROP_UINT8("minor", MPCIState, minor, 0),
> +    DEFINE_PROP_UINT8("model", MPCIState, model, 0x41),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void mcb_pci_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> +
> +    k->realize = mcb_pci_realize;
> +    k->exit = mcb_pci_unrealize;
> +    k->vendor_id = PCI_VENDOR_ID_MEN;
> +    k->device_id = PCI_DEVICE_ID_MEN_MCBPCI;
> +    k->class_id = PCI_CLASS_BRIDGE_OTHER;
> +
> +    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> +    dc->desc = "MEN Chameleon Bus over PCI";
> +    dc->vmsd = &vmstate_mcb_pci;
> +    device_class_set_props(dc, mcb_pci_props);
> +}
> +
> +static const TypeInfo mcb_pci_info = {
> +    .name = TYPE_MCB_PCI,
> +    .parent = TYPE_PCI_DEVICE,
> +    .instance_size = sizeof(MPCIState),
> +    .class_init = mcb_pci_class_init,
> +    .interfaces = (InterfaceInfo[]) {
> +        { INTERFACE_PCIE_DEVICE },
> +        { }
> +    },
> +};
> +
> +static void mcb_pci_register_types(void)
> +{
> +    type_register(&mcb_pci_info);
> +}
> +type_init(mcb_pci_register_types);
> diff --git a/hw/mcb/meson.build b/hw/mcb/meson.build
> index a385edc07c..4e1a0f0cdb 100644
> --- a/hw/mcb/meson.build
> +++ b/hw/mcb/meson.build
> @@ -1 +1,2 @@
>  softmmu_ss.add(when: 'CONFIG_MCB', if_true: files('mcb.c'))
> +softmmu_ss.add(when: 'CONFIG_MCB_PCI', if_true: files('mcb-pci.c'))
> --
> 2.39.2
>
>