On 10/12/23 18:09, Volodymyr Babchuk wrote:
> diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
> index 33db58580c..40d1a07ada 100644
> --- a/xen/drivers/vpci/header.c
> +++ b/xen/drivers/vpci/header.c
> @@ -477,6 +477,74 @@ static void cf_check bar_write(
> pci_conf_write32(pdev->sbdf, reg, val);
> }
>
> +static void cf_check guest_bar_write(const struct pci_dev *pdev,
> + unsigned int reg, uint32_t val, void
> *data)
> +{
> + struct vpci_bar *bar = data;
> + bool hi = false;
> + uint64_t guest_addr = bar->guest_addr;
This initialization is using the initial value of bar ...
> +
> + if ( bar->type == VPCI_BAR_MEM64_HI )
> + {
> + ASSERT(reg > PCI_BASE_ADDRESS_0);
> + bar--;
... but here bar is decremented ...
> + hi = true;
> + }
> + else
> + {
> + val &= PCI_BASE_ADDRESS_MEM_MASK;
> + }
> +
... so to ensure a consistent state, the initialization should be moved here.
> + guest_addr &= ~(0xffffffffULL << (hi ? 32 : 0));
> + guest_addr |= (uint64_t)val << (hi ? 32 : 0);
> +
> + /* Allow guest to size BAR correctly */
> + guest_addr &= ~(bar->size - 1);
> +
> + /*
> + * Make sure that the guest set address has the same page offset
> + * as the physical address on the host or otherwise things won't work as
> + * expected.
> + */
> + if ( guest_addr != ~(bar->size -1 ) &&
Should this sizing check only apply to the lower 32 bits, or take "hi" into
account?
For reference, it may be helpful to see an example sequence of a Linux domU
sizing a 64 bit BAR. I instrumented guest_bar_write() to print the raw/initial
val argument, and guest_bar_read() to print the final reg_val:
(XEN) drivers/vpci/header.c:guest_bar_read d1 0000:01:00.0 reg 0x10 val
0xe0100004
(XEN) drivers/vpci/header.c:guest_bar_write d1 0000:01:00.0 reg 0x10 val
0xffffffff
(XEN) drivers/vpci/header.c:guest_bar_read d1 0000:01:00.0 reg 0x10 val
0xffffc004
(XEN) drivers/vpci/header.c:guest_bar_write d1 0000:01:00.0 reg 0x10 val
0xe0100004
(XEN) drivers/vpci/header.c:guest_bar_read d1 0000:01:00.0 reg 0x14 val 0x0
(hi)
(XEN) drivers/vpci/header.c:guest_bar_write d1 0000:01:00.0 reg 0x14 val
0xffffffff (hi)
(XEN) drivers/vpci/header.c:guest_bar_read d1 0000:01:00.0 reg 0x14 val
0xffffffff (hi)
(XEN) drivers/vpci/header.c:guest_bar_write d1 0000:01:00.0 reg 0x14 val 0x0
(hi)
(XEN) drivers/vpci/header.c:guest_bar_write d1 0000:01:00.0 reg 0x10 val
0x23000004
(XEN) drivers/vpci/header.c:guest_bar_read d1 0000:01:00.0 reg 0x10 val
0x23000004
(XEN) drivers/vpci/header.c:guest_bar_write d1 0000:01:00.0 reg 0x14 val 0x0
(hi)
(XEN) drivers/vpci/header.c:guest_bar_read d1 0000:01:00.0 reg 0x14 val 0x0
(hi)
> + PAGE_OFFSET(guest_addr) != PAGE_OFFSET(bar->addr) )
> + {
> + gprintk(XENLOG_WARNING,
> + "%pp: ignored BAR %zu write attempting to change page
> offset\n",
> + &pdev->sbdf, bar - pdev->vpci->header.bars + hi);
> + return;
> + }
> +
> + bar->guest_addr = guest_addr;
> +}
