| Message ID | 1404834132-15847-1-git-send-email-stefano.stabellini@eu.citrix.com |
|---|---|
| State | New |
| Headers | show |
On Tue, 2014-07-08 at 16:42 +0100, Stefano Stabellini wrote: > The flag tells us that the hypervisor maps a grant page to guest > physical address == machine address of the page in addition to the > normal grant mapping address. It is needed to properly issue cache > maintenance operation at the completion of a DMA operation involving a > foreign grant. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > --- > arch/arm/xen/enlighten.c | 6 ++++++ > include/xen/interface/features.h | 3 +++ > 2 files changed, 9 insertions(+) > > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c > index b96723e..ee3135a 100644 > --- a/arch/arm/xen/enlighten.c > +++ b/arch/arm/xen/enlighten.c > @@ -262,6 +262,12 @@ static int __init xen_guest_init(void) > xen_domain_type = XEN_HVM_DOMAIN; > > xen_setup_features(); > + > + if (!xen_feature(XENFEAT_grant_map_11)) { > + pr_warn("Please upgrade your Xen.\n" > + "If your platform has any non-coherent DMA devices, they won't work properly.\n"); > + } Unfortunately this isn't quite complete. On a system where all devices are behind an SMMU then we would want to be able to disable the 1:1 workaround, which in turn would imply disabling this feature flag too (since it is no longer necessary and also impossible to implement in that case). Ian.
On Tue, 8 Jul 2014, Ian Campbell wrote: > On Tue, 2014-07-08 at 16:42 +0100, Stefano Stabellini wrote: > > The flag tells us that the hypervisor maps a grant page to guest > > physical address == machine address of the page in addition to the > > normal grant mapping address. It is needed to properly issue cache > > maintenance operation at the completion of a DMA operation involving a > > foreign grant. > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > --- > > arch/arm/xen/enlighten.c | 6 ++++++ > > include/xen/interface/features.h | 3 +++ > > 2 files changed, 9 insertions(+) > > > > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c > > index b96723e..ee3135a 100644 > > --- a/arch/arm/xen/enlighten.c > > +++ b/arch/arm/xen/enlighten.c > > @@ -262,6 +262,12 @@ static int __init xen_guest_init(void) > > xen_domain_type = XEN_HVM_DOMAIN; > > > > xen_setup_features(); > > + > > + if (!xen_feature(XENFEAT_grant_map_11)) { > > + pr_warn("Please upgrade your Xen.\n" > > + "If your platform has any non-coherent DMA devices, they won't work properly.\n"); > > + } > > Unfortunately this isn't quite complete. On a system where all devices > are behind an SMMU then we would want to be able to disable the 1:1 > workaround, which in turn would imply disabling this feature flag too > (since it is no longer necessary and also impossible to implement in > that case). That is true, but in such a system we would have to tell the kernel that DMAing is safe, so this will turn into: if (!xen_feature(XENFEAT_grant_map_11) && !xen_feature(XENFEAT_safe_dma)) { pr_warn("Please upgrade your Xen.\n" "If your platform has any non-coherent DMA devices, they won't work properly.\n"); }
On Tue, 2014-07-08 at 16:54 +0100, Stefano Stabellini wrote: > On Tue, 8 Jul 2014, Ian Campbell wrote: > > On Tue, 2014-07-08 at 16:42 +0100, Stefano Stabellini wrote: > > > The flag tells us that the hypervisor maps a grant page to guest > > > physical address == machine address of the page in addition to the > > > normal grant mapping address. It is needed to properly issue cache > > > maintenance operation at the completion of a DMA operation involving a > > > foreign grant. > > > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > > --- > > > arch/arm/xen/enlighten.c | 6 ++++++ > > > include/xen/interface/features.h | 3 +++ > > > 2 files changed, 9 insertions(+) > > > > > > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c > > > index b96723e..ee3135a 100644 > > > --- a/arch/arm/xen/enlighten.c > > > +++ b/arch/arm/xen/enlighten.c > > > @@ -262,6 +262,12 @@ static int __init xen_guest_init(void) > > > xen_domain_type = XEN_HVM_DOMAIN; > > > > > > xen_setup_features(); > > > + > > > + if (!xen_feature(XENFEAT_grant_map_11)) { > > > + pr_warn("Please upgrade your Xen.\n" > > > + "If your platform has any non-coherent DMA devices, they won't work properly.\n"); > > > + } > > > > Unfortunately this isn't quite complete. On a system where all devices > > are behind an SMMU then we would want to be able to disable the 1:1 > > workaround, which in turn would imply disabling this feature flag too > > (since it is no longer necessary and also impossible to implement in > > that case). > > That is true, but in such a system we would have to tell the kernel that > DMAing is safe, so this will turn into: Oh right, yes. Good then ;-) > > if (!xen_feature(XENFEAT_grant_map_11) && > !xen_feature(XENFEAT_safe_dma)) { > pr_warn("Please upgrade your Xen.\n" > "If your platform has any non-coherent DMA devices, they won't work properly.\n"); > }
On 08/07/14 16:42, Stefano Stabellini wrote: > The flag tells us that the hypervisor maps a grant page to guest > physical address == machine address of the page in addition to the > normal grant mapping address. It is needed to properly issue cache > maintenance operation at the completion of a DMA operation involving a > foreign grant. [...] > +/* Xen also maps grant references at pfn = mfn */ > +#define XENFEAT_grant_map_11 12 I keep reading this as "grant map eleven". I think you've used this abbreviation else where so you should probably keep it as-is. I might have picked XENFEAT_grant_map_identity. David
Hi Ian and Stefano, On 07/08/2014 04:59 PM, Ian Campbell wrote: > On Tue, 2014-07-08 at 16:54 +0100, Stefano Stabellini wrote: >> On Tue, 8 Jul 2014, Ian Campbell wrote: >>> On Tue, 2014-07-08 at 16:42 +0100, Stefano Stabellini wrote: >>>> The flag tells us that the hypervisor maps a grant page to guest >>>> physical address == machine address of the page in addition to the >>>> normal grant mapping address. It is needed to properly issue cache >>>> maintenance operation at the completion of a DMA operation involving a >>>> foreign grant. >>>> >>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> >>>> --- >>>> arch/arm/xen/enlighten.c | 6 ++++++ >>>> include/xen/interface/features.h | 3 +++ >>>> 2 files changed, 9 insertions(+) >>>> >>>> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c >>>> index b96723e..ee3135a 100644 >>>> --- a/arch/arm/xen/enlighten.c >>>> +++ b/arch/arm/xen/enlighten.c >>>> @@ -262,6 +262,12 @@ static int __init xen_guest_init(void) >>>> xen_domain_type = XEN_HVM_DOMAIN; >>>> >>>> xen_setup_features(); >>>> + >>>> + if (!xen_feature(XENFEAT_grant_map_11)) { >>>> + pr_warn("Please upgrade your Xen.\n" >>>> + "If your platform has any non-coherent DMA devices, they won't work properly.\n"); >>>> + } >>> >>> Unfortunately this isn't quite complete. On a system where all devices >>> are behind an SMMU then we would want to be able to disable the 1:1 >>> workaround, which in turn would imply disabling this feature flag too >>> (since it is no longer necessary and also impossible to implement in >>> that case). >> >> That is true, but in such a system we would have to tell the kernel that >> DMAing is safe, so this will turn into: > > Oh right, yes. Good then ;-) FWIW, I've sent a patch series a couple ago to avoid using swiotlb when the device is protected (see https://patches.linaro.org/25070/). I should take time to rework properly and send a new version. Regards,
diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c index b96723e..ee3135a 100644 --- a/arch/arm/xen/enlighten.c +++ b/arch/arm/xen/enlighten.c @@ -262,6 +262,12 @@ static int __init xen_guest_init(void) xen_domain_type = XEN_HVM_DOMAIN; xen_setup_features(); + + if (!xen_feature(XENFEAT_grant_map_11)) { + pr_warn("Please upgrade your Xen.\n" + "If your platform has any non-coherent DMA devices, they won't work properly.\n"); + } + if (xen_feature(XENFEAT_dom0)) xen_start_info->flags |= SIF_INITDOMAIN|SIF_PRIVILEGED; else diff --git a/include/xen/interface/features.h b/include/xen/interface/features.h index 131a6cc..6517dd5 100644 --- a/include/xen/interface/features.h +++ b/include/xen/interface/features.h @@ -53,6 +53,9 @@ /* operation as Dom0 is supported */ #define XENFEAT_dom0 11 +/* Xen also maps grant references at pfn = mfn */ +#define XENFEAT_grant_map_11 12 + #define XENFEAT_NR_SUBMAPS 1 #endif /* __XEN_PUBLIC_FEATURES_H__ */
The flag tells us that the hypervisor maps a grant page to guest physical address == machine address of the page in addition to the normal grant mapping address. It is needed to properly issue cache maintenance operation at the completion of a DMA operation involving a foreign grant. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> --- arch/arm/xen/enlighten.c | 6 ++++++ include/xen/interface/features.h | 3 +++ 2 files changed, 9 insertions(+)