Re: [PATCH v2 3/3] xen/vpci: msix: move read/write call to MSI-X PBA entry to arch file

From: "Roger Pau Monné" <roger.pau@citrix.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: Rahul Singh <rahul.singh@arm.com>, <bertrand.marquis@arm.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>, Wei Liu <wl@xen.org>,
	<xen-devel@lists.xenproject.org>
Subject: Re: [PATCH v2 3/3] xen/vpci: msix: move read/write call to MSI-X PBA entry to arch file
Date: Fri, 25 Feb 2022 09:21:50 +0100	[thread overview]
Message-ID: <YhiRnmgajgeat/fP@Air-de-Roger> (raw)
In-Reply-To: <c8f54e83-95c9-04aa-d788-da755e284c42@suse.com>

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.