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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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.

Jan

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