On 02/03/2022 08:10, Jan Beulich wrote:
> On 01.03.2022 15:58, Andrew Cooper wrote:
>> On 25/02/2022 08:24, Jan Beulich wrote:
>>> On 22.02.2022 12:47, Andrew Cooper wrote:
>>>> --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
>>>> +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
>>>> @@ -628,7 +628,7 @@ static void cf_check amd_dump_page_tables(struct
>>>> domain *d)
>>>> hd->arch.amd.paging_mode, 0, 0);
>>>> }
>>>>
>>>> -static const struct iommu_ops __initconstrel _iommu_ops = {
>>>> +static const struct iommu_ops __initconst_cf_clobber _iommu_ops = {
>>> Following my initcall related remark on x86'es time.c I'm afraid I don't
>>> see how this and ...
>>>
>>>> @@ -2794,7 +2793,7 @@ static int __init cf_check
>>>> intel_iommu_quarantine_init(struct domain *d)
>>>> return rc;
>>>> }
>>>>
>>>> -static struct iommu_ops __initdata vtd_ops = {
>>>> +static const struct iommu_ops __initconst_cf_clobber vtd_ops = {
>>> ... this actually works. But I guess I must be overlooking something, as
>>> I'm sure that you did test the change.
>>>
>>> Both ops structures reference a function, through .adjust_irq_affinities,
>>> which isn't __init but which is used (besides here) for an initcall. With
>>> the ENDBR removed by the time initcalls are run, these should cause #CP.
>> This doesn't explode because the indirect calls are resolved to direct
>> calls before the ENDBR's are clobbered to NOP4.
> I'm afraid I don't understand: The problematic call is in do_initcalls():
>
> for ( call = __presmp_initcall_end; call < __initcall_end; call++ )
> (*call)();
>
> I don't see how this could be converted to a direct call.
Oh. iov_adjust_irq_affinities()'s double use is hiding here.
The safety rule for cf_clobber is that there must not be any
non-alt-called callers. We need to fix it:
diff --git a/xen/drivers/passthrough/amd/iommu_init.c
b/xen/drivers/passthrough/amd/iommu_init.c
index 657c7f619a51..b1af5085efda 100644
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -831,7 +831,12 @@ int cf_check iov_adjust_irq_affinities(void)
return 0;
}
-__initcall(iov_adjust_irq_affinities);
+
+int cf_check __init initcall_iov_adjust_irq_affinities(void)
+{
+ return iommu_call(&iommu_ops, adjust_irq_affinities);
+}
+__initcall(initcall_iov_adjust_irq_affinities);
/*
* Family15h Model 10h-1fh erratum 746 (IOMMU Logging May Stall
Translations)
> Afaics only pre-SMP initcalls are safe in this regard: do_presmp_initcalls()
> is called immediately ahead of alternative_branches().
>
> Isn't this (previously?) working related to your "x86/spec-ctrl: Disable
> retpolines with CET-IBT"?
No. It's because AMD CPUs don't have CET-IBT at this juncture, and will
never encounter a faulting situation.
This is exactly what the UD1 adjustment in Linux are intended to spot,
because that would cause all AMD hardware to explode, not just the
IBT-enabled ones.
~Andrew
