Re: [PATCH v2] x86/PVH: modify permission checking in hwdom_fixup_p2m()

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

On Tue, Jul 15, 2025 at 12:47:15PM +0200, Jan Beulich wrote:
> On 15.07.2025 12:09, Roger Pau Monné wrote:
> > On Mon, Jul 14, 2025 at 06:09:27PM +0200, Jan Beulich wrote:
> >> We're generally striving to minimize behavioral differences between PV
> >> and PVH Dom0. Using is_memory_hole() in the PVH case looks quite a bit
> >> weaker to me, compared to the page ownership check done in the PV case.
> >> Change checking accordingly.
> >>
> >> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> > 
> > Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> 
> Thanks.
> 
> > Kind of unrelated to this specific patch, but what's our opinion on
> > turning on pf-fixup by default before the release?
> 
> As far as the patch here goes, the relationship is very tight. I came to
> make this patch only while investigating whether we couldn't have Dom0
> report the resource (MMIO) ranges early enough for us to not even need
> such fixing-up. Sadly, as per [1] that turned out pretty much impossible.
> Which means that while I'm still pretty hesitant of us doing something
> like this by default, I can't currently see a way around doing so. Hence
> perhaps yes, we may want (or even need) to turn this on by default.

Sorry, wanted to reply to your previous commit alternative approach
email, but got distracted with something else and forgot about it.

While I won't be opposed to having a way for dom0 to notify extra MMIO
regions it wants added to the p2m, I think this is likely too much
fuzz.  For example for FreeBSD I wouldn't consider adding such logic
to the kernel, simply because I think it's likely to be too intrusive,
and would rather rely on pf-fixup.  Overall the amount of p2m fixups
that Xen ends up doing is always fairly small (I usually see maybe 4
pages tops), and only as result of ACPI related accesses.  IMO it's an
acceptable compromise to map those as individual 4K pages.

I would only consider the alternative approach of using a hypercall if
we saw big regions being mapped by pf-fixup, because in that case it
would better be using p2m superpage(s).

I think we want to enable pf-fixup by default at some point, the
question is whether you would consider it appropriate to do now.
Given it's limited to PVH dom0 only, I think we should enable for this
release already.

Thanks, Roger.