Re: XSA-378 fixes breaking PVH Dom0 (was: [xen-4.15-testing test] 164495: regressions - FAIL)

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.08.2021 15:29, Jan Beulich wrote:
> On 27.08.2021 08:52, osstest service owner wrote:
>> flight 164495 xen-4.15-testing real [real]
>> flight 164509 xen-4.15-testing real-retest [real]
>> http://logs.test-lab.xenproject.org/osstest/logs/164495/
>> http://logs.test-lab.xenproject.org/osstest/logs/164509/
>>
>> Regressions :-(
>>
>> Tests which did not succeed and are blocking,
>> including tests which could not be run:
>>  test-amd64-amd64-dom0pvh-xl-amd  8 xen-boot              fail REGR. vs. 
>> 163759
>>  test-amd64-amd64-dom0pvh-xl-intel  8 xen-boot            fail REGR. vs. 
>> 163759
> 
> This is fallout from XSA-378. During Dom0 setup we first call
> iommu_hwdom_init(), which maps reserved regions (p2m_access_rw).
> Later map_mmio_regions() gets used to identity-map the low first
> Mb. This, using set_mmio_p2m_entry(), establishes default-access
> mappings (p2m_access_rwx).
> 
> Hence even if relaxing the logic in set_typed_p2m_entry() to
> 
>     if ( p2m_is_special(ot) )
>     {
>         gfn_unlock(p2m, gfn, order);
>         if ( mfn_eq(mfn, omfn) && gfn_p2mt == ot && access == a )
>             return 0;
>         domain_crash(d);
>         return -EPERM;
>     }
> 
> we're still in trouble (because the two access types don't match)
> when there is any reserved region below 1Mb.
> 
> One approach would be to avoid blindly mapping the low first Mb,
> and to instead honor mappings which are already there. Or the
> opposite - avoid mapping anything from arch_iommu_hwdom_init()
> which is below 1Mb. (Other mappings down the call tree from
> pvh_setup_acpi() imo would then also need adjusting, to avoid
> redundant mapping attempts of space below 1Mb. At least RSDP is
> known to possibly live there on various systems.)
> 
> Another approach could be to stop passing ->default_access from
> set_mmio_p2m_entry() to set_typed_p2m_entry(). (And I think the
> same should go for set_foreign_p2m_entry()). At the very least
> right now it makes no sense at all to make RWX mappings there,
> except when mapping PCI device ROMs. But of course reducing
> permissions always comes with a (however large or small) risk of
> regressions.
> 
> While I think the latter aspect wants improving in any event,
> right now I'm leaning towards the "opposite" variant of the
> former. I'll draft a patch along these lines at least to see if
> it helps, or if there is yet more fallout.

There is more fallout - with the initial issue addressed as
described I'm now hitting another similar domain_crash() in
guest_physmap_add_entry(). There's no question there whether to
check that old and new mappings match - they are different. Here
PVH Dom0 setup really does what the final XSA-378 patch is
intended to disallow: It produces MMIO mappings to then replace
some (or really most) by RAM ones. This means I'll have to
further adjust how pvh_populate_p2m() works. This will have to
wait until after the weekend, sorry.

Jan