[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH RFC server v2 06/11] vfio-user: handle PCI config space acces
From: |
Stefan Hajnoczi |
Subject: |
Re: [PATCH RFC server v2 06/11] vfio-user: handle PCI config space accesses |
Date: |
Mon, 13 Sep 2021 13:13:33 +0100 |
On Fri, Sep 10, 2021 at 04:22:56PM +0000, Jag Raman wrote:
>
>
> > On Sep 9, 2021, at 3:27 AM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >
> > 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.
>
> I’ll collect more details on this one, trying to wrap my head around it.
>
> Concerning config space writes, it doesn’t look like we need to
> perform any conversion as the store operation automatically happens
> in the CPU’s native format when we do something like the following:
> PCIDevice->config[addr] = val;
PCIDevice->config is uint8_t*. Endianness isn't an issue with single
byte accesses.
>
> Concerning config read, I observed that pci_default_read_config()
> performs le32_to_cpu() conversion. May be we should not rely on
> it doing the conversion.
Yes, ->config_read() returns a host-endian 32-bit value and
->config_write() receives a host-endian 32-bit value (it has a
bit-shifting loop that implicitly handles endianness conversion).
Stefan
signature.asc
Description: PGP signature