Re: [PATCH v5 1/3] x86/iommu: remove regions not to be mapped

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 25/01/2024 1:13 pm, Jan Beulich wrote:
> On 25.01.2024 13:55, Andrew Cooper wrote:
>> On 25/01/2024 12:37 pm, Roger Pau Monné wrote:
>>> On Thu, Jan 25, 2024 at 12:13:01PM +0100, Jan Beulich wrote:
>>>> On 25.01.2024 09:47, Roger Pau Monné wrote:
>>>>> On Thu, Jan 25, 2024 at 09:34:40AM +0100, Jan Beulich wrote:
>>>>>> On 24.01.2024 18:29, Roger Pau Monne wrote:
>>>>>>> --- a/xen/arch/x86/hvm/io.c
>>>>>>> +++ b/xen/arch/x86/hvm/io.c
>>>>>>> @@ -369,6 +369,22 @@ bool vpci_is_mmcfg_address(const struct domain *d, 
>>>>>>> paddr_t addr)
>>>>>>>      return vpci_mmcfg_find(d, addr);
>>>>>>>  }
>>>>>>>  
>>>>>>> +int __hwdom_init vpci_subtract_mmcfg(const struct domain *d, struct 
>>>>>>> rangeset *r)
>>>>>>> +{
>>>>>>> +    const struct hvm_mmcfg *mmcfg;
>>>>>>> +
>>>>>>> +    list_for_each_entry ( mmcfg, &d->arch.hvm.mmcfg_regions, next )
>>>>>>> +    {
>>>>>>> +        int rc = rangeset_remove_range(r, PFN_DOWN(mmcfg->addr),
>>>>>>> +                                       PFN_DOWN(mmcfg->addr + 
>>>>>>> mmcfg->size - 1));
>>>>>> Along the lines of this, ...
>>>>>>
>>>>>>> --- a/xen/arch/x86/setup.c
>>>>>>> +++ b/xen/arch/x86/setup.c
>>>>>>> @@ -2138,6 +2138,54 @@ int __hwdom_init xen_in_range(unsigned long mfn)
>>>>>>>      return 0;
>>>>>>>  }
>>>>>>>  
>>>>>>> +int __hwdom_init remove_xen_ranges(struct rangeset *r)
>>>>>>> +{
>>>>>>> +    paddr_t start, end;
>>>>>>> +    int rc;
>>>>>>> +
>>>>>>> +    /* S3 resume code (and other real mode trampoline code) */
>>>>>>> +    rc = rangeset_remove_range(r, 
>>>>>>> PFN_DOWN(bootsym_phys(trampoline_start)),
>>>>>>> +                               PFN_DOWN(bootsym_phys(trampoline_end)) 
>>>>>>> - 1);
>>>>>> ... did you perhaps mean
>>>>>>
>>>>>>                                PFN_DOWN(bootsym_phys(trampoline_end) - 
>>>>>> 1));
>>>>>>
>>>>>> here (and then similarly below, except there the difference is benign I
>>>>>> think, for the labels being page-aligned)?
>>>>> They are all page aligned, so I didn't care much,  but now that you
>>>>> point it might be safer to do the subtraction from the address instead
>>>>> of the frame number, just in case.
>>>> Hmm, no, for me neither trampoline_end nor trampoline_start are page
>>>> aligned. While bootsym_phys(trampoline_start) is, I don't think
>>>> bootsym_phys(trampoline_end) normally would be (it might only be by
>>>> coincidence).
>>> Oh, so it had been a coincidence of the build I was using I guess then.
>> trampoline_start has to be page aligned because of constraints from SIPI
>> and S3 (cant remember which one is the 4k constraint, but it's in the
>> comments).
> What you're talking about is the copy of the trampoline code/data in
> low memory. trampoline_{start,end} themselves point into the Xen image.

True, but we're operating on bootsym_phys(trampoline_start) which had
better be aligned.

We hard-code (by virtue of only filling in 1 single 4k PTE in the
pagetables) that the AP trampoline is 4k.

The range here should be 4k only too, or we're (falsely) marking lowmem
adjacent to the AP trampoline as a Xen range when it's not.

~Andrew