[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2] x86/P2M: synchronize fast and slow paths of p2m_get_page_from_gfn()


  • To: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Thu, 27 Mar 2025 11:54:09 +0100
  • Autocrypt: addr=jbeulich@xxxxxxxx; keydata= xsDiBFk3nEQRBADAEaSw6zC/EJkiwGPXbWtPxl2xCdSoeepS07jW8UgcHNurfHvUzogEq5xk hu507c3BarVjyWCJOylMNR98Yd8VqD9UfmX0Hb8/BrA+Hl6/DB/eqGptrf4BSRwcZQM32aZK 7Pj2XbGWIUrZrd70x1eAP9QE3P79Y2oLrsCgbZJfEwCgvz9JjGmQqQkRiTVzlZVCJYcyGGsD /0tbFCzD2h20ahe8rC1gbb3K3qk+LpBtvjBu1RY9drYk0NymiGbJWZgab6t1jM7sk2vuf0Py O9Hf9XBmK0uE9IgMaiCpc32XV9oASz6UJebwkX+zF2jG5I1BfnO9g7KlotcA/v5ClMjgo6Gl MDY4HxoSRu3i1cqqSDtVlt+AOVBJBACrZcnHAUSuCXBPy0jOlBhxPqRWv6ND4c9PH1xjQ3NP nxJuMBS8rnNg22uyfAgmBKNLpLgAGVRMZGaGoJObGf72s6TeIqKJo/LtggAS9qAUiuKVnygo 3wjfkS9A3DRO+SpU7JqWdsveeIQyeyEJ/8PTowmSQLakF+3fote9ybzd880fSmFuIEJldWxp Y2ggPGpiZXVsaWNoQHN1c2UuY29tPsJgBBMRAgAgBQJZN5xEAhsDBgsJCAcDAgQVAggDBBYC AwECHgECF4AACgkQoDSui/t3IH4J+wCfQ5jHdEjCRHj23O/5ttg9r9OIruwAn3103WUITZee e7Sbg12UgcQ5lv7SzsFNBFk3nEQQCACCuTjCjFOUdi5Nm244F+78kLghRcin/awv+IrTcIWF hUpSs1Y91iQQ7KItirz5uwCPlwejSJDQJLIS+QtJHaXDXeV6NI0Uef1hP20+y8qydDiVkv6l IreXjTb7DvksRgJNvCkWtYnlS3mYvQ9NzS9PhyALWbXnH6sIJd2O9lKS1Mrfq+y0IXCP10eS FFGg+Av3IQeFatkJAyju0PPthyTqxSI4lZYuJVPknzgaeuJv/2NccrPvmeDg6Coe7ZIeQ8Yj t0ARxu2xytAkkLCel1Lz1WLmwLstV30g80nkgZf/wr+/BXJW/oIvRlonUkxv+IbBM3dX2OV8 AmRv1ySWPTP7AAMFB/9PQK/VtlNUJvg8GXj9ootzrteGfVZVVT4XBJkfwBcpC/XcPzldjv+3 HYudvpdNK3lLujXeA5fLOH+Z/G9WBc5pFVSMocI71I8bT8lIAzreg0WvkWg5V2WZsUMlnDL9 mpwIGFhlbM3gfDMs7MPMu8YQRFVdUvtSpaAs8OFfGQ0ia3LGZcjA6Ik2+xcqscEJzNH+qh8V m5jjp28yZgaqTaRbg3M/+MTbMpicpZuqF4rnB0AQD12/3BNWDR6bmh+EkYSMcEIpQmBM51qM EKYTQGybRCjpnKHGOxG0rfFY1085mBDZCH5Kx0cl0HVJuQKC+dV2ZY5AqjcKwAxpE75MLFkr wkkEGBECAAkFAlk3nEQCGwwACgkQoDSui/t3IH7nnwCfcJWUDUFKdCsBH/E5d+0ZnMQi+G0A nAuWpQkjM1ASeQwSHEeAWPgskBQL
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Delivery-date: Thu, 27 Mar 2025 10:54:14 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 27.03.2025 11:38, Roger Pau Monné wrote:
> On Thu, Mar 27, 2025 at 10:24:02AM +0100, Jan Beulich wrote:
>> On 27.03.2025 10:00, Roger Pau Monné wrote:
>>> On Tue, Mar 25, 2025 at 08:17:04AM +0100, Jan Beulich wrote:
>>>> Handling of both grants and foreign pages was different between the two
>>>> paths.
>>>>
>>>> While permitting access to grants would be desirable, doing so would
>>>> require more involved handling; undo that for the time being. In
>>>> particular the page reference obtained would prevent the owning domain
>>>> from changing e.g. the page's type (after the grantee has released the
>>>> last reference of the grant). Instead perhaps another reference on the
>>>> grant would need obtaining. Which in turn would require determining
>>>> which grant that was.
>>>>
>>>> Foreign pages in any event need permitting on both paths.
>>>
>>> I've been wondering about this, and AFAICT the PV equivalent of this
>>> function (the tail of get_page_from_gfn()) is much more limited in
>>> that it just allows access to domain owned RAM or MMIO that has a
>>> backing page (I expect mostly for the low 1MB?).
>>>
>>> However for HVM domains we allow to take references to foreign mapped
>>> pages and taking references to MMIO pages is not permitted.
>>>
>>> Should the tail of get_page_from_gfn() also be adjusted to make use of
>>> the newly introduced get_page_from_mfn_and_type(), thus unifying the
>>> logic for both PV and HVM?
>>
>> There's no (proper) use of P2M types in PV, so I don't think using this
>> function is viable for PV. In particular we'd never observe p2m_foreign
>> in PV, if I'm not mistaken.
> 
> Indeed, p2m types are just occasionally faked for PV, like in
> get_page_from_gfn().
> 
>>> Could possibly be done in a separate change, I'm just trying to
>>> understand why we have this seemingly different handling for PV vs
>>> HVM.
>>
>> The difference is because access to foreign pages was hacked in to work
>> for PVH in a much different (and imo yet more hacky) way than it was
>> made work far longer ago for PV. The crucial part of that is in
>> get_page_from_l1e(), so get_page_from_gfn() isn't that relevant there
>> (I think).
> 
> OK.  I'm kind of surprised we need foreign handling for HVM and not
> for PV in get_page_from_gfn(), as even with specific handling of
> foreign pages in get_page_from_l1e(), the former is used in a lot of
> places that don't seem obviously tied or gated to a call to
> get_page_from_l1e().
> 
> For example the XEN_DOMCTL_getpageframeinfo3 usage of
> get_page_from_gfn() will have different results when used against a
> foreign page depending on whether the caller is a PV or an HVM
> domain.

Which imo is an unintended side effect of how foreign page support was added
to p2m_get_page_from_gfn(). Considering what XEN_DOMCTL_getpageframeinfo3 is
used for, I don't think foreign pages were intended to ever be observed there.
A guest controlling another guest isn't assumed to be migrated, aiui.

I expect more uses of get_page_from_gfn() can be found where foreign pages
better wouldn't have "success" returned. Yet any of this is mere fallout from
some callers apparently needing that case to work, and hence why logic was
hacked into there. Maybe get_page_from_gfn() should take another P2M_FOREIGN
flag, which only callers caring about foreign pages would set?

Imo none of this is directly relevant for the patch at hand here - fast and
slow paths differing is a problem no matter what. In fact I'm surprised no
issues were ever reported that could be attributed back to that anomaly.

Jan



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.