Re: [PATCH v2.2 8/7] x86/IOMMU: Use altcall, and __initconst_cf_clobber

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

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"? With full retpoline, there wouldn't be an
indirect branch, but RET. But with JMP or LFENCE thunks this ought to
fault (already before depending on thunk selection, but unconditionally
now that your change was committed), I would think.

Jan