On Thu, Feb 24, 2022 at 04:18:31PM +0100, Jan Beulich wrote:
> On 15.02.2022 16:25, Rahul Singh wrote:
> > @@ -170,31 +170,7 @@ bool vpci_msix_read(struct vpci_msix *msix, unsigned
> > long addr,
> > return true;
> >
> > if ( VMSIX_ADDR_IN_RANGE(addr, msix->pdev->vpci, VPCI_MSIX_PBA) )
> > - {
> > - /*
> > - * Access to PBA.
> > - *
> > - * TODO: note that this relies on having the PBA identity mapped
> > to the
> > - * guest address space. If this changes the address will need to be
> > - * translated.
> > - */
> > - switch ( len )
> > - {
> > - case 4:
> > - *data = readl(addr);
> > - break;
> > -
> > - case 8:
> > - *data = readq(addr);
> > - break;
> > -
> > - default:
> > - ASSERT_UNREACHABLE();
> > - break;
> > - }
>
> ... in the comment ahead of this switch() (and the assumption is likely
> wrong for DomU).
>
> But then, Roger: What "identity mapped" is meant here? Surely not GVA ->
> GPA, but rather GPA -> HPA? The address here is a guest physical one,
> but read{l,q}() act on (host) virtual addresses. This would have been
> easier to notice as wrong if read{l,q}() weren't allowing unsigned long
> arguments to be passed to them.
Urg, indeed, thanks for noticing. I will send a patch to fix this
right now.
Roger.
