On Thu, Sep 30, 2021 at 10:52:17AM +0300, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <[email protected]>
>
> Emulate guest BAR register values: this allows creating a guest view
> of the registers and emulates size and properties probe as it is done
> during PCI device enumeration by the guest.
>
> ROM BAR is only handled for the hardware domain and for guest domains
> there is a stub: at the moment PCI expansion ROM is x86 only, so it
> might not be used by other architectures without emulating x86. Other
> use-cases may include using that expansion ROM before Xen boots, hence
> no emulation is needed in Xen itself. Or when a guest wants to use the
> ROM code which seems to be rare.
>
> Signed-off-by: Oleksandr Andrushchenko <[email protected]>
> Reviewed-by: Michal Orzel <[email protected]>
> ---
> Since v1:
> - re-work guest read/write to be much simpler and do more work on write
> than read which is expected to be called more frequently
> - removed one too obvious comment
>
> Signed-off-by: Oleksandr Andrushchenko <[email protected]>
> ---
> xen/drivers/vpci/header.c | 30 +++++++++++++++++++++++++++++-
> xen/include/xen/vpci.h | 3 +++
> 2 files changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
> index 1ce98795fcca..ec4d215f36ff 100644
> --- a/xen/drivers/vpci/header.c
> +++ b/xen/drivers/vpci/header.c
> @@ -400,12 +400,38 @@ static void bar_write(const struct pci_dev *pdev,
> unsigned int reg,
> static void guest_bar_write(const struct pci_dev *pdev, unsigned int reg,
> uint32_t val, void *data)
> {
> + struct vpci_bar *bar = data;
> + bool hi = false;
> +
> + if ( bar->type == VPCI_BAR_MEM64_HI )
> + {
> + ASSERT(reg > PCI_BASE_ADDRESS_0);
> + bar--;
> + hi = true;
> + }
> + else
> + {
> + val &= PCI_BASE_ADDRESS_MEM_MASK;
> + val |= bar->type == VPCI_BAR_MEM32 ? PCI_BASE_ADDRESS_MEM_TYPE_32
> + : PCI_BASE_ADDRESS_MEM_TYPE_64;
> + val |= bar->prefetchable ? PCI_BASE_ADDRESS_MEM_PREFETCH : 0;
> + }
> +
> + bar->guest_addr &= ~(0xffffffffull << (hi ? 32 : 0));
> + bar->guest_addr |= (uint64_t)val << (hi ? 32 : 0);
> +
> + bar->guest_addr &= ~(bar->size - 1) | ~PCI_BASE_ADDRESS_MEM_MASK;
> }
>
> static uint32_t guest_bar_read(const struct pci_dev *pdev, unsigned int reg,
> void *data)
> {
> - return 0xffffffff;
> + const struct vpci_bar *bar = data;
> +
> + if ( bar->type == VPCI_BAR_MEM64_HI )
> + return bar->guest_addr >> 32;
> +
> + return bar->guest_addr;
I think this is missing a check for whether the BAR is the high part
of a 64bit one? Ie:
struct vpci_bar *bar = data;
bool hi = false;
if ( bar->type == VPCI_BAR_MEM64_HI )
{
ASSERT(reg > PCI_BASE_ADDRESS_0);
bar--;
hi = true;
}
return bar->guest_addr >> (hi ? 32 : 0);
Or else when accessing the high part of a 64bit BAR you will always
return 0s as it hasn't been setup by guest_bar_write.
Thanks, Roger.
