On Fri, Aug 27, 2021 at 01:53:25PM -0400, Jagannathan Raman wrote:
> +static ssize_t vfu_object_cfg_access(vfu_ctx_t *vfu_ctx, char * const buf,
> +                                     size_t count, loff_t offset,
> +                                     const bool is_write)
> +{
> +    VfuObject *o = vfu_get_private(vfu_ctx);
> +    uint32_t pci_access_width = sizeof(uint32_t);
> +    size_t bytes = count;
> +    uint32_t val = 0;
> +    char *ptr = buf;
> +    int len;
> +
> +    while (bytes > 0) {
> +        len = (bytes > pci_access_width) ? pci_access_width : bytes;
> +        if (is_write) {
> +            memcpy(&val, ptr, len);
> +            pci_default_write_config(PCI_DEVICE(o->pci_dev),
> +                                     offset, val, len);
> +            trace_vfu_cfg_write(offset, val);
> +        } else {
> +            val = pci_default_read_config(PCI_DEVICE(o->pci_dev),
> +                                          offset, len);
> +            memcpy(ptr, &val, len);

pci_default_read_config() returns a host-endian 32-bit value. This code
looks wrong because it copies different bytes on big- and little-endian
hosts.

> +            trace_vfu_cfg_read(offset, val);
> +        }

Why call pci_default_read/write_config() directly instead of
pci_dev->config_read/write()?