Re: [Xen-devel] [PATCH v2 2/2] x86/io: move the list of guest to machine IO ports out of domain_iommu

[Roger Pau Monne <roger.pau@xxxxxxxxxx>]

On Tue, Apr 04, 2017 at 07:39:29AM -0600, Jan Beulich wrote:
> >>> On 04.04.17 at 15:23, <andrew.cooper3@xxxxxxxxxx> wrote:
> > On 04/04/17 12:31, Jan Beulich wrote:
> >>>>> On 04.04.17 at 13:06, <roger.pau@xxxxxxxxxx> wrote:
> >>> On Wed, Mar 29, 2017 at 11:49:53AM +0100, Roger Pau Monne wrote:
> >>>> On Wed, Mar 29, 2017 at 11:41:40AM +0100, Andrew Cooper wrote:
> >>>>> On 28/03/17 14:12, Roger Pau Monne wrote:
> >>>>>> There's no reason to store that list inside of the domain_iommu 
> >>>>>> struct, the
> >>>>>> forwarding of guest IO ports into machine IO ports is not tied to the 
> > presence
> >>>>>> of an IOMMU.
> >>>>>>
> >>>>>> Move it inside of the hvm_domain struct instead.
> >>>>>>
> >>>>>> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> >>>>> Actually, on second thoughts, I rescind my R-by.
> >>>>>
> >>>>> This breaks PV guests, which must not use state in the hvm half of the
> >>>>> union.
> >>>> I'm extremely confused now, AFAICT the g2m_ioport_list list is only used 
> >>>> by the
> >>>> g2m_portio_* handlers, and those handlers already make use of HVM only 
> >>>> fields
> >>>> (ie: hvm_vcpu_io).
> >>> Can this be committed?
> >> As said on irc, I think was posted after the last posting date, so you'd
> >> need to obtain an ack from Julien.
> >>
> >>> I understand there's a previous regression here, but
> >>> this patches don't make it any worse AFAICT, and they clarify the 
> >>> nomenclature.
> >> I don't see the "previous regression": The handlers can be invoked
> >> only for HVM guests. What is not HVM-specific is the domctl, and
> >> there is where you introduce a regression.
> > 
> > It is certainly the case that in the past, you could use this domctl to
> > pass ISA ports through to PV guests, and that definitely regressed as
> > some point in the past.  ISTR Ian (cc'd) fielded a question to this
> > effect from Debian.
> 
> How would that have worked? Making ports (in)accessible is done
> via XEN_DOMCTL_ioport_permission. Going back to 3.2.3 I actually
> find that what is now named dom_iommu() was domain_hvm_iommu()
> back then, and it wasn't entirely mis-named (i.e. the field was in
> arch.hvm_domain). So I think all that's needed is adding an
> is_hvm_domain() to the domctl handling.

I can either do that, or move the list to arch_domain instead of hvm_domain.
Either way it's not going to make it work for PV guests, but it's going to
prevent PV guests from writing to hvm_domain.

Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel