[PATCH] IOMMU: iommu_use_hap_pt() implies CONFIG_HVM

Jan Beulich posted 1 patch 2 months, 3 weeks ago
Failed in applying to current master (apply log)
[PATCH] IOMMU: iommu_use_hap_pt() implies CONFIG_HVM
Posted by Jan Beulich 2 months, 3 weeks ago
Allow the compiler a little more room on DCE by moving the compile-time-
constant condition into the predicate (from the one place where it was
added in an open-coded fashion for XSA-450).

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -438,7 +438,7 @@ static paddr_t domain_pgd_maddr(struct d
 
     if ( pgd_maddr )
         /* nothing */;
-    else if ( IS_ENABLED(CONFIG_HVM) && iommu_use_hap_pt(d) )
+    else if ( iommu_use_hap_pt(d) )
     {
         pagetable_t pgt = p2m_get_pagetable(p2m_get_hostp2m(d));
 
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -381,7 +381,8 @@ struct domain_iommu {
 #define iommu_clear_feature(d, f) clear_bit(f, dom_iommu(d)->features)
 
 /* Are we using the domain P2M table as its IOMMU pagetable? */
-#define iommu_use_hap_pt(d)       (dom_iommu(d)->hap_pt_share)
+#define iommu_use_hap_pt(d)       (IS_ENABLED(CONFIG_HVM) && \
+                                   dom_iommu(d)->hap_pt_share)
 
 /* Does the IOMMU pagetable need to be kept synchronized with the P2M */
 #ifdef CONFIG_HAS_PASSTHROUGH
Re: [PATCH] IOMMU: iommu_use_hap_pt() implies CONFIG_HVM
Posted by Andrew Cooper 2 months, 3 weeks ago
On 31/01/2024 9:20 am, Jan Beulich wrote:
> Allow the compiler a little more room on DCE by moving the compile-time-
> constant condition into the predicate (from the one place where it was
> added in an open-coded fashion for XSA-450).
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

I'm surprised that's happy compiling if it's actually doing anything at
all, given that it's an unguarded variable reference.
Re: [PATCH] IOMMU: iommu_use_hap_pt() implies CONFIG_HVM
Posted by Jan Beulich 2 months, 3 weeks ago
On 31.01.2024 11:40, Andrew Cooper wrote:
> On 31/01/2024 9:20 am, Jan Beulich wrote:
>> Allow the compiler a little more room on DCE by moving the compile-time-
>> constant condition into the predicate (from the one place where it was
>> added in an open-coded fashion for XSA-450).
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

Thanks, but ...

> I'm surprised that's happy compiling if it's actually doing anything at
> all, given that it's an unguarded variable reference.

... I'm afraid I don't understand: What "unguarded variable reference"
are you seeing me add? Even if you were talking about the hap_pt_share
struct field (which was accessed before as well) - that's part of
struct domain_iommu no matter what .config has.

Jan
Re: [PATCH] IOMMU: iommu_use_hap_pt() implies CONFIG_HVM
Posted by Andrew Cooper 2 months, 3 weeks ago
On 31/01/2024 11:09 am, Jan Beulich wrote:
> On 31.01.2024 11:40, Andrew Cooper wrote:
>> On 31/01/2024 9:20 am, Jan Beulich wrote:
>>> Allow the compiler a little more room on DCE by moving the compile-time-
>>> constant condition into the predicate (from the one place where it was
>>> added in an open-coded fashion for XSA-450).
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Thanks, but ...
>
>> I'm surprised that's happy compiling if it's actually doing anything at
>> all, given that it's an unguarded variable reference.
> ... I'm afraid I don't understand: What "unguarded variable reference"
> are you seeing me add? Even if you were talking about the hap_pt_share
> struct field (which was accessed before as well) - that's part of
> struct domain_iommu no matter what .config has.

Ah.  That will be why it compiles.

Fine.

~Andrew