ATS and dependent features

ATS and dependent features

[Jan Beulich]

All,

while investigating an issue with some device not working under Xen
without "ats=off", I started wondering whether it is correct to allow
the Dom0 kernel concurrent control over ATS, PRI, and PASID - it
would seem to me that with Xen controlling the IOMMU, it should
also have exclusive control over the enabling of those features.

Thanks for any comments in this regard,
Jan

Re: ATS and dependent features

[Zhang, Xiantao]

ATS should be a host feature controlled by iommu, and I don't think dom0 can control  it from Xen's architecture.   Perhaps we need to forbid dom0 controlling these features. 
Xiantao

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: Tuesday, November 27, 2012 5:42 PM
> To: Zhang, Xiantao; xen-devel
> Subject: ATS and dependent features
> 
> All,
> 
> while investigating an issue with some device not working under Xen without
> "ats=off", I started wondering whether it is correct to allow the Dom0 kernel
> concurrent control over ATS, PRI, and PASID - it would seem to me that with
> Xen controlling the IOMMU, it should also have exclusive control over the
> enabling of those features.
> 
> Thanks for any comments in this regard,
> Jan

Re: ATS and dependent features

[Jan Beulich]

>>> On 29.11.12 at 02:07, "Zhang, Xiantao" <xiantao.zhang@intel.com> wrote:
> ATS should be a host feature controlled by iommu, and I don't think dom0 can 
> control  it from Xen's architecture.

"Can" or "should"? Because from all I can tell it currently clearly
does.

>   Perhaps we need to forbid dom0 controlling these features. 

That's what I was trying to find out. How to properly do that
with pv-ops I don't know; in our forward ported kernels I can
simply make the respective config options conditional upon !XEN.

Jan

>> From: Jan Beulich [mailto:JBeulich@suse.com]
>> while investigating an issue with some device not working under Xen without
>> "ats=off", I started wondering whether it is correct to allow the Dom0 
> kernel
>> concurrent control over ATS, PRI, and PASID - it would seem to me that with
>> Xen controlling the IOMMU, it should also have exclusive control over the
>> enabling of those features.
>> 
>> Thanks for any comments in this regard,
>> Jan

Re: ATS and dependent features

[Zhang, Xiantao]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: Thursday, November 29, 2012 4:01 PM
> To: Zhang, Xiantao
> Cc: xen-devel
> Subject: RE: ATS and dependent features
> 
> >>> On 29.11.12 at 02:07, "Zhang, Xiantao" <xiantao.zhang@intel.com>
> wrote:
> > ATS should be a host feature controlled by iommu, and I don't think
> > dom0 can control  it from Xen's architecture.
> 
> "Can" or "should"? Because from all I can tell it currently clearly does.

I mean Xen shouldn't  allow these capabilities can be detected by dom0.  If it does, we need to fix it. 

> >   Perhaps we need to forbid dom0 controlling these features.
> 
> That's what I was trying to find out. How to properly do that with pv-ops I
> don't know; in our forward ported kernels I can simply make the respective
> config options conditional upon !XEN. 

As to the clean solution, Xen should intercept all the accesses to the device's PCI config space from dom0, and emulates the related registers.  In your simple solution, it maybe break the kernel running with these features on native hardware.  If adopts pv_ops method to solve such corner issues, it may be not acceptable by upstream kernel.   
Xiantao  

> >> From: Jan Beulich [mailto:JBeulich@suse.com] while investigating an
> >> issue with some device not working under Xen without "ats=off", I
> >> started wondering whether it is correct to allow the Dom0
> > kernel
> >> concurrent control over ATS, PRI, and PASID - it would seem to me
> >> that with Xen controlling the IOMMU, it should also have exclusive
> >> control over the enabling of those features.
> >>
> >> Thanks for any comments in this regard, Jan
> 
> 

Re: ATS and dependent features

[Jan Beulich]

>>> On 29.11.12 at 10:19, "Zhang, Xiantao" <xiantao.zhang@intel.com> wrote:

> 
>> -----Original Message-----
>> From: Jan Beulich [mailto:JBeulich@suse.com]
>> Sent: Thursday, November 29, 2012 4:01 PM
>> To: Zhang, Xiantao
>> Cc: xen-devel
>> Subject: RE: ATS and dependent features
>> 
>> >>> On 29.11.12 at 02:07, "Zhang, Xiantao" <xiantao.zhang@intel.com>
>> wrote:
>> > ATS should be a host feature controlled by iommu, and I don't think
>> > dom0 can control  it from Xen's architecture.
>> 
>> "Can" or "should"? Because from all I can tell it currently clearly does.
> 
> I mean Xen shouldn't  allow these capabilities can be detected by dom0.  If 
> it does, we need to fix it. 

It sort of hides it - all callers sit in the kernel's IOMMU code, and
IOMMU detection is being prevented. So it looks like the code is
simply dead when running on top of Xen.

>> >   Perhaps we need to forbid dom0 controlling these features.
>> 
>> That's what I was trying to find out. How to properly do that with pv-ops I
>> don't know; in our forward ported kernels I can simply make the respective
>> config options conditional upon !XEN. 
> 
> As to the clean solution, Xen should intercept all the accesses to the 
> device's PCI config space from dom0, and emulates the related registers.

Why? That would e.g. make lspci completely unreliable.

Jan

Re: ATS and dependent features

[Zhang, Xiantao]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: Thursday, November 29, 2012 5:28 PM
> To: Zhang, Xiantao
> Cc: xen-devel
> Subject: RE: ATS and dependent features
> 
> >>> On 29.11.12 at 10:19, "Zhang, Xiantao" <xiantao.zhang@intel.com>
> wrote:
> 
> >
> >> -----Original Message-----
> >> From: Jan Beulich [mailto:JBeulich@suse.com]
> >> Sent: Thursday, November 29, 2012 4:01 PM
> >> To: Zhang, Xiantao
> >> Cc: xen-devel
> >> Subject: RE: ATS and dependent features
> >>
> >> >>> On 29.11.12 at 02:07, "Zhang, Xiantao" <xiantao.zhang@intel.com>
> >> wrote:
> >> > ATS should be a host feature controlled by iommu, and I don't think
> >> > dom0 can control  it from Xen's architecture.
> >>
> >> "Can" or "should"? Because from all I can tell it currently clearly does.
> >
> > I mean Xen shouldn't  allow these capabilities can be detected by
> > dom0.  If it does, we need to fix it.
> 
> It sort of hides it - all callers sit in the kernel's IOMMU code, and IOMMU
> detection is being prevented. So it looks like the code is simply dead when
> running on top of Xen.

I'm curious why dom0's !Xen kernel option for these features can solve the issue you met. 

> >> >   Perhaps we need to forbid dom0 controlling these features.
> >>
> >> That's what I was trying to find out. How to properly do that with
> >> pv-ops I don't know; in our forward ported kernels I can simply make
> >> the respective config options conditional upon !XEN.
> >
> > As to the clean solution, Xen should intercept all the accesses to the
> > device's PCI config space from dom0, and emulates the related registers.
> 
> Why? That would e.g. make lspci completely unreliable.

yes,  if these capabilities are completely virtualized, lspci should be  break. 
Xiantao 

Re: ATS and dependent features

[Jan Beulich]

>>> On 30.11.12 at 13:29, "Zhang, Xiantao" <xiantao.zhang@intel.com> wrote:

> 
>> -----Original Message-----
>> From: Jan Beulich [mailto:JBeulich@suse.com]
>> Sent: Thursday, November 29, 2012 5:28 PM
>> To: Zhang, Xiantao
>> Cc: xen-devel
>> Subject: RE: ATS and dependent features
>> 
>> >>> On 29.11.12 at 10:19, "Zhang, Xiantao" <xiantao.zhang@intel.com>
>> wrote:
>> 
>> >
>> >> -----Original Message-----
>> >> From: Jan Beulich [mailto:JBeulich@suse.com]
>> >> Sent: Thursday, November 29, 2012 4:01 PM
>> >> To: Zhang, Xiantao
>> >> Cc: xen-devel
>> >> Subject: RE: ATS and dependent features
>> >>
>> >> >>> On 29.11.12 at 02:07, "Zhang, Xiantao" <xiantao.zhang@intel.com>
>> >> wrote:
>> >> > ATS should be a host feature controlled by iommu, and I don't think
>> >> > dom0 can control  it from Xen's architecture.
>> >>
>> >> "Can" or "should"? Because from all I can tell it currently clearly does.
>> >
>> > I mean Xen shouldn't  allow these capabilities can be detected by
>> > dom0.  If it does, we need to fix it.
>> 
>> It sort of hides it - all callers sit in the kernel's IOMMU code, and IOMMU
>> detection is being prevented. So it looks like the code is simply dead when
>> running on top of Xen.
> 
> I'm curious why dom0's !Xen kernel option for these features can solve the 
> issue you met. 

It doesn't "solve" the problem in that sense: As said, the code in
question only has callers in IOMMU code, which itself is dependent
on !XEN in our kernels (just to make clear - I'm talking about forward
ported kernels here, not pv-ops ones). So upstream probably just
has to live with that code being dead (at the moment, when run on
top of Xen) and take the risk of there appearing a caller elsewhere.
In our kernels, by making these options also dependent upon !XEN,
we can then actually detect (and actively deal with) an eventual
new caller elsewhere in the code, thus eliminating any risk of bad
interaction between Dom0 and Xen.

Jan

Re: ATS and dependent features

[Zhang, Xiantao]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: Monday, December 03, 2012 3:55 PM
> To: Zhang, Xiantao
> Cc: xen-devel
> Subject: RE: ATS and dependent features
> 
> >>> On 30.11.12 at 13:29, "Zhang, Xiantao" <xiantao.zhang@intel.com>
> wrote:
> 
> >
> >> -----Original Message-----
> >> From: Jan Beulich [mailto:JBeulich@suse.com]
> >> Sent: Thursday, November 29, 2012 5:28 PM
> >> To: Zhang, Xiantao
> >> Cc: xen-devel
> >> Subject: RE: ATS and dependent features
> >>
> >> >>> On 29.11.12 at 10:19, "Zhang, Xiantao" <xiantao.zhang@intel.com>
> >> wrote:
> >>
> >> >
> >> >> -----Original Message-----
> >> >> From: Jan Beulich [mailto:JBeulich@suse.com]
> >> >> Sent: Thursday, November 29, 2012 4:01 PM
> >> >> To: Zhang, Xiantao
> >> >> Cc: xen-devel
> >> >> Subject: RE: ATS and dependent features
> >> >>
> >> >> >>> On 29.11.12 at 02:07, "Zhang, Xiantao"
> >> >> >>> <xiantao.zhang@intel.com>
> >> >> wrote:
> >> >> > ATS should be a host feature controlled by iommu, and I don't
> >> >> > think
> >> >> > dom0 can control  it from Xen's architecture.
> >> >>
> >> >> "Can" or "should"? Because from all I can tell it currently clearly does.
> >> >
> >> > I mean Xen shouldn't  allow these capabilities can be detected by
> >> > dom0.  If it does, we need to fix it.
> >>
> >> It sort of hides it - all callers sit in the kernel's IOMMU code, and
> >> IOMMU detection is being prevented. So it looks like the code is
> >> simply dead when running on top of Xen.
> >
> > I'm curious why dom0's !Xen kernel option for these features can solve
> > the issue you met.
> 
> It doesn't "solve" the problem in that sense: As said, the code in question
> only has callers in IOMMU code, which itself is dependent on !XEN in our
> kernels (just to make clear - I'm talking about forward ported kernels here,
> not pv-ops ones). So upstream probably just has to live with that code being
> dead (at the moment, when run on top of Xen) and take the risk of there
> appearing a caller elsewhere.
> In our kernels, by making these options also dependent upon !XEN, we can
> then actually detect (and actively deal with) an eventual new caller
> elsewhere in the code, thus eliminating any risk of bad interaction between
> Dom0 and Xen.

I think !Xen you are talking is a compile option, so this kernel can only used for dom0 and can't run on native with these features enabled ?  If don't need to keep the kernel running on native hardware, I think it is fine. 
Xiantao 

Re: ATS and dependent features

[Jan Beulich]

>>> On 04.12.12 at 02:29, "Zhang, Xiantao" <xiantao.zhang@intel.com> wrote:

> 
>> -----Original Message-----
>> From: Jan Beulich [mailto:JBeulich@suse.com]
>> Sent: Monday, December 03, 2012 3:55 PM
>> To: Zhang, Xiantao
>> Cc: xen-devel
>> Subject: RE: ATS and dependent features
>> 
>> >>> On 30.11.12 at 13:29, "Zhang, Xiantao" <xiantao.zhang@intel.com>
>> wrote:
>> 
>> >
>> >> -----Original Message-----
>> >> From: Jan Beulich [mailto:JBeulich@suse.com]
>> >> Sent: Thursday, November 29, 2012 5:28 PM
>> >> To: Zhang, Xiantao
>> >> Cc: xen-devel
>> >> Subject: RE: ATS and dependent features
>> >>
>> >> >>> On 29.11.12 at 10:19, "Zhang, Xiantao" <xiantao.zhang@intel.com>
>> >> wrote:
>> >>
>> >> >
>> >> >> -----Original Message-----
>> >> >> From: Jan Beulich [mailto:JBeulich@suse.com]
>> >> >> Sent: Thursday, November 29, 2012 4:01 PM
>> >> >> To: Zhang, Xiantao
>> >> >> Cc: xen-devel
>> >> >> Subject: RE: ATS and dependent features
>> >> >>
>> >> >> >>> On 29.11.12 at 02:07, "Zhang, Xiantao"
>> >> >> >>> <xiantao.zhang@intel.com>
>> >> >> wrote:
>> >> >> > ATS should be a host feature controlled by iommu, and I don't
>> >> >> > think
>> >> >> > dom0 can control  it from Xen's architecture.
>> >> >>
>> >> >> "Can" or "should"? Because from all I can tell it currently clearly does.
>> >> >
>> >> > I mean Xen shouldn't  allow these capabilities can be detected by
>> >> > dom0.  If it does, we need to fix it.
>> >>
>> >> It sort of hides it - all callers sit in the kernel's IOMMU code, and
>> >> IOMMU detection is being prevented. So it looks like the code is
>> >> simply dead when running on top of Xen.
>> >
>> > I'm curious why dom0's !Xen kernel option for these features can solve
>> > the issue you met.
>> 
>> It doesn't "solve" the problem in that sense: As said, the code in question
>> only has callers in IOMMU code, which itself is dependent on !XEN in our
>> kernels (just to make clear - I'm talking about forward ported kernels here,
>> not pv-ops ones). So upstream probably just has to live with that code being
>> dead (at the moment, when run on top of Xen) and take the risk of there
>> appearing a caller elsewhere.
>> In our kernels, by making these options also dependent upon !XEN, we can
>> then actually detect (and actively deal with) an eventual new caller
>> elsewhere in the code, thus eliminating any risk of bad interaction between
>> Dom0 and Xen.
> 
> I think !Xen you are talking is a compile option, so this kernel can only 
> used for dom0 and can't run on native with these features enabled ?  If don't 
> need to keep the kernel running on native hardware, I think it is fine. 

Yes, as said - this is for our forward ported kernel. Whether (and if
so how) the pv-ops one can add a similar safeguard I can't tell (and
doubt).

Jan

Re: ATS and dependent features

[Konrad Rzeszutek Wilk]

On Tue, Dec 04, 2012 at 08:12:25AM +0000, Jan Beulich wrote:
> >>> On 04.12.12 at 02:29, "Zhang, Xiantao" <xiantao.zhang@intel.com> wrote:
> 
> > 
> >> -----Original Message-----
> >> From: Jan Beulich [mailto:JBeulich@suse.com]
> >> Sent: Monday, December 03, 2012 3:55 PM
> >> To: Zhang, Xiantao
> >> Cc: xen-devel
> >> Subject: RE: ATS and dependent features
> >> 
> >> >>> On 30.11.12 at 13:29, "Zhang, Xiantao" <xiantao.zhang@intel.com>
> >> wrote:
> >> 
> >> >
> >> >> -----Original Message-----
> >> >> From: Jan Beulich [mailto:JBeulich@suse.com]
> >> >> Sent: Thursday, November 29, 2012 5:28 PM
> >> >> To: Zhang, Xiantao
> >> >> Cc: xen-devel
> >> >> Subject: RE: ATS and dependent features
> >> >>
> >> >> >>> On 29.11.12 at 10:19, "Zhang, Xiantao" <xiantao.zhang@intel.com>
> >> >> wrote:
> >> >>
> >> >> >
> >> >> >> -----Original Message-----
> >> >> >> From: Jan Beulich [mailto:JBeulich@suse.com]
> >> >> >> Sent: Thursday, November 29, 2012 4:01 PM
> >> >> >> To: Zhang, Xiantao
> >> >> >> Cc: xen-devel
> >> >> >> Subject: RE: ATS and dependent features
> >> >> >>
> >> >> >> >>> On 29.11.12 at 02:07, "Zhang, Xiantao"
> >> >> >> >>> <xiantao.zhang@intel.com>
> >> >> >> wrote:
> >> >> >> > ATS should be a host feature controlled by iommu, and I don't
> >> >> >> > think
> >> >> >> > dom0 can control  it from Xen's architecture.
> >> >> >>
> >> >> >> "Can" or "should"? Because from all I can tell it currently clearly does.
> >> >> >
> >> >> > I mean Xen shouldn't  allow these capabilities can be detected by
> >> >> > dom0.  If it does, we need to fix it.
> >> >>
> >> >> It sort of hides it - all callers sit in the kernel's IOMMU code, and
> >> >> IOMMU detection is being prevented. So it looks like the code is
> >> >> simply dead when running on top of Xen.
> >> >
> >> > I'm curious why dom0's !Xen kernel option for these features can solve
> >> > the issue you met.
> >> 
> >> It doesn't "solve" the problem in that sense: As said, the code in question
> >> only has callers in IOMMU code, which itself is dependent on !XEN in our
> >> kernels (just to make clear - I'm talking about forward ported kernels here,
> >> not pv-ops ones). So upstream probably just has to live with that code being
> >> dead (at the moment, when run on top of Xen) and take the risk of there
> >> appearing a caller elsewhere.
> >> In our kernels, by making these options also dependent upon !XEN, we can
> >> then actually detect (and actively deal with) an eventual new caller
> >> elsewhere in the code, thus eliminating any risk of bad interaction between
> >> Dom0 and Xen.
> > 
> > I think !Xen you are talking is a compile option, so this kernel can only 
> > used for dom0 and can't run on native with these features enabled ?  If don't 
> > need to keep the kernel running on native hardware, I think it is fine. 
> 
> Yes, as said - this is for our forward ported kernel. Whether (and if
> so how) the pv-ops one can add a similar safeguard I can't tell (and
> doubt).

Right, in the upstream kernel that is not going to work. But I am a bit
confused - this code (pci_enable_ats) looks to be called only from the
intel and amd iommu code. Aren't those blacklisted (so DMAR MADT
is overwritten and AMD PCI device is witheld) so the calls aren't going
to be executed?

> 
> Jan
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
> 

Re: ATS and dependent features

[Jan Beulich]

>>> On 04.12.12 at 16:13, Konrad Rzeszutek Wilk <konrad@kernel.org> wrote:
> On Tue, Dec 04, 2012 at 08:12:25AM +0000, Jan Beulich wrote:
>> >>> On 04.12.12 at 02:29, "Zhang, Xiantao" <xiantao.zhang@intel.com> wrote:
>> 
>> > 
>> >> -----Original Message-----
>> >> From: Jan Beulich [mailto:JBeulich@suse.com]
>> >> Sent: Monday, December 03, 2012 3:55 PM
>> >> To: Zhang, Xiantao
>> >> Cc: xen-devel
>> >> Subject: RE: ATS and dependent features
>> >> 
>> >> >>> On 30.11.12 at 13:29, "Zhang, Xiantao" <xiantao.zhang@intel.com>
>> >> wrote:
>> >> 
>> >> >
>> >> >> -----Original Message-----
>> >> >> From: Jan Beulich [mailto:JBeulich@suse.com]
>> >> >> Sent: Thursday, November 29, 2012 5:28 PM
>> >> >> To: Zhang, Xiantao
>> >> >> Cc: xen-devel
>> >> >> Subject: RE: ATS and dependent features
>> >> >>
>> >> >> >>> On 29.11.12 at 10:19, "Zhang, Xiantao" <xiantao.zhang@intel.com>
>> >> >> wrote:
>> >> >>
>> >> >> >
>> >> >> >> -----Original Message-----
>> >> >> >> From: Jan Beulich [mailto:JBeulich@suse.com]
>> >> >> >> Sent: Thursday, November 29, 2012 4:01 PM
>> >> >> >> To: Zhang, Xiantao
>> >> >> >> Cc: xen-devel
>> >> >> >> Subject: RE: ATS and dependent features
>> >> >> >>
>> >> >> >> >>> On 29.11.12 at 02:07, "Zhang, Xiantao"
>> >> >> >> >>> <xiantao.zhang@intel.com>
>> >> >> >> wrote:
>> >> >> >> > ATS should be a host feature controlled by iommu, and I don't
>> >> >> >> > think
>> >> >> >> > dom0 can control  it from Xen's architecture.
>> >> >> >>
>> >> >> >> "Can" or "should"? Because from all I can tell it currently clearly does.
>> >> >> >
>> >> >> > I mean Xen shouldn't  allow these capabilities can be detected by
>> >> >> > dom0.  If it does, we need to fix it.
>> >> >>
>> >> >> It sort of hides it - all callers sit in the kernel's IOMMU code, and
>> >> >> IOMMU detection is being prevented. So it looks like the code is
>> >> >> simply dead when running on top of Xen.
>> >> >
>> >> > I'm curious why dom0's !Xen kernel option for these features can solve
>> >> > the issue you met.
>> >> 
>> >> It doesn't "solve" the problem in that sense: As said, the code in question
>> >> only has callers in IOMMU code, which itself is dependent on !XEN in our
>> >> kernels (just to make clear - I'm talking about forward ported kernels here,
>> >> not pv-ops ones). So upstream probably just has to live with that code being
>> >> dead (at the moment, when run on top of Xen) and take the risk of there
>> >> appearing a caller elsewhere.
>> >> In our kernels, by making these options also dependent upon !XEN, we can
>> >> then actually detect (and actively deal with) an eventual new caller
>> >> elsewhere in the code, thus eliminating any risk of bad interaction between
>> >> Dom0 and Xen.
>> > 
>> > I think !Xen you are talking is a compile option, so this kernel can only 
>> > used for dom0 and can't run on native with these features enabled ?  If 
> don't 
>> > need to keep the kernel running on native hardware, I think it is fine. 
>> 
>> Yes, as said - this is for our forward ported kernel. Whether (and if
>> so how) the pv-ops one can add a similar safeguard I can't tell (and
>> doubt).
> 
> Right, in the upstream kernel that is not going to work. But I am a bit
> confused - this code (pci_enable_ats) looks to be called only from the
> intel and amd iommu code. Aren't those blacklisted (so DMAR MADT
> is overwritten and AMD PCI device is witheld) so the calls aren't going
> to be executed?

That's right. But the code being there means you wouldn't notice
(at build time) if some other caller appeared (which would need
fixing).

Jan