Re: [Xen-devel] [PATCH v2 02/11] ioreq: terminate cf8 handling at hypervisor level

From: Paul Durrant <Paul.Durrant@citrix.com>
To: Roger Pau Monne <roger.pau@citrix.com>
Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>, Wei Liu <wl@xen.org>,
	Jan Beulich <jbeulich@suse.com>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Subject: Re: [Xen-devel] [PATCH v2 02/11] ioreq: terminate cf8 handling at hypervisor level
Date: Wed, 4 Sep 2019 13:56:10 +0000	[thread overview]
Message-ID: <92821b5ab3b04714a83617eff89b753d@AMSPEX02CL03.citrite.net> (raw)
In-Reply-To: <20190904133957.2g2sxfvhfbf7n3x3@Air-de-Roger>

> -----Original Message-----
> From: Roger Pau Monne <roger.pau@citrix.com>
> Sent: 04 September 2019 14:40
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; xen-devel@lists.xenproject.org; Jan Beulich
> <jbeulich@suse.com>; Wei Liu <wl@xen.org>
> Subject: Re: [PATCH v2 02/11] ioreq: terminate cf8 handling at hypervisor level
> 
> On Wed, Sep 04, 2019 at 11:46:24AM +0200, Paul Durrant wrote:
> > > -----Original Message-----
> > > From: Roger Pau Monne <roger.pau@citrix.com>
> > > Sent: 04 September 2019 08:49
> > > To: Andrew Cooper <Andrew.Cooper3@citrix.com>
> > > Cc: xen-devel@lists.xenproject.org; Paul Durrant <Paul.Durrant@citrix.com>; Jan Beulich
> > > <jbeulich@suse.com>; Wei Liu <wl@xen.org>
> > > Subject: Re: [PATCH v2 02/11] ioreq: terminate cf8 handling at hypervisor level
> > >
> > > On Tue, Sep 03, 2019 at 06:13:59PM +0100, Andrew Cooper wrote:
> > > > On 03/09/2019 17:14, Roger Pau Monne wrote:
> > > > > diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
> > > > > index 692b710b02..69652e1080 100644
> > > > > --- a/xen/arch/x86/hvm/ioreq.c
> > > > > +++ b/xen/arch/x86/hvm/ioreq.c
> > > > > @@ -1015,6 +1015,12 @@ int hvm_map_io_range_to_ioreq_server(struct domain *d, ioservid_t id,
> > > > >      switch ( type )
> > > > >      {
> > > > >      case XEN_DMOP_IO_RANGE_PORT:
> > > > > +        rc = -EINVAL;
> > > > > +        /* PCI config space accesses are handled internally. */
> > > > > +        if ( start <= 0xcf8 + 8 && 0xcf8 <= end )
> > > > > +            goto out;
> > > > > +        else
> > > > > +            /* fallthrough. */
> > > >
> > > > You need to drop the else, or it may still trigger warnings.
> > >
> > > Yes, my mistake. The else branch is not needed.
> > >
> > > > Furthermore, qemu registers cf8-cff so I think you need some fix-ups
> > > > there first before throwing errors back here.
> > >
> > > The version of QEMU I have doesn't seem to register 0xcf8 or 0xcfc,
> > > there are no errors on the log and QEMU seems to work just fine.
> > >
> > > I always assumed QEMU was getting accesses to cf8/cfc forwarded
> > > because it was the default device model, and everything not trapped by
> > > Xen would be forwarded to it. This default device model behaviour was
> > > removed by Paul some time ago, and now QEMU registers explicitly which
> > > IO accesses it wants to trap.
> >
> > Yes, it used to need them to work correctly as a default emulator. However, we don't generally stop
> an external emulator from registering ranges that are handled by emulation directly in Xen (e.g.
> pmtimer) so I don't think you need special-case these ports.
> 
> That's right, so I guess I would just remove that check (and the one
> introduced for MCFG regions). We also don't check whether any other
> ioreq server has already registered a range.

That's right... it's a last-one-wins game. We could decide to change this in future, but it is quite convenient for testing purposes.

  Paul

> 
> Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel