Re: [PATCH] x86/mm: do not mark IO regions as Xen heap

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Sep 21, 2020 at 05:49:45PM +0200, Jan Beulich wrote:
> On 21.09.2020 17:35, Roger Pau Monné wrote:
> > On Mon, Sep 21, 2020 at 04:22:26PM +0200, Jan Beulich wrote:
> >> On 10.09.2020 15:35, Roger Pau Monne wrote:
> >>> arch_init_memory will treat all the gaps on the physical memory map
> >>> between RAM regions as MMIO and use share_xen_page_with_guest in order
> >>> to assign them to dom_io. This has the side effect of setting the Xen
> >>> heap flag on such pages, and thus is_special_page would then return
> >>> true which is an issue in epte_get_entry_emt because such pages will
> >>> be forced to use write-back cache attributes.
> >>>
> >>> Fix this by introducing a new helper to assign the MMIO regions to
> >>> dom_io without setting the Xen heap flag on the pages, so that
> >>> is_special_page will return false and the pages won't be forced to use
> >>> write-back cache attributes.
> >>>
> >>> Fixes: 81fd0d3ca4b2cd ('x86/hvm: simplify 'mmio_direct' check in 
> >>> epte_get_entry_emt()')
> >>
> >> Is this really the correct commit? I had this queued for backport,
> >> and ended up discarding it from the queue based on this tag, just
> >> to then noticing that I remembered correctly that I did backport
> >> ca24b2ffdbd9 ("x86/hvm: set 'ipat' in EPT for special pages") to
> >> the stable trees already. Isn't it _this_ commit which the change
> >> here fixes?
> > 
> > My bisection pointed to that exact commit as the one that broke my PVH
> > dom0 setup, so yes, I'm quite sure that's the commit at least on the
> > box that I've bisected it.
> > 
> > ca24b2ffdbd9 was still fine because previous to the is_special_page
> > check loop there was still the `if ( direct_mmio ) ...` handling,
> > which made all MMIO regions except for the APIC access page forcefully
> > have it's cache attributes set to UC.
> 
> Ah yes, I see - thanks. Makes me less sure whether the patch
> here really wants backporting.

As long as 81fd0d3ca4b2cd is not backported, then I would argue to not
backport this either. I don't see much benefit in backporting this
alone, and the risk of introducing a non intended functionality change
as a result of not marking MMIO pages as Xen heap is possible.

Thanks, Roger.