On 21.08.25 12:05, Lorenzo Stoakes wrote:
On Thu, Aug 21, 2025 at 11:30:43AM +0200, David Hildenbrand wrote:
I will add this xen/apply_to_page_range() thing to my TODOs, which atm
would invovle changing these drivers to use vmf_insert_pfn_prot() instead.
Busy today (want to reply to Christian) but
a) Re: performance, we would want something like
vmf_insert_pfns_prot(), similar to vm_insert_pages(), to bulk-insert
multiple PFNs.
b) Re: PAT, we'll have to figure out why PAT information is wrong here
(was there no previous PAT reservation from the driver?), but IF we
really have to override, we'd want a way to tell
vmf_insert_pfn_prot() to force the selected caching mode.
Ack, ok good that we have a feasible way forward.
FYI, spoke to Peter off-list and he mentioned he had a more general series
to get rid of this kind of [ab]use of apply_to_page_range() (see [0]), I
gather he hasn't the time to resurrect but perhaps one of us can at some
point?
Perhaps we need a shorter term fix to _this_ issue (which involves not
using this interface), and then follow it up with an adaptation of the
below?
We need to understand why PAT would be wrong and why it would even be ok
to ignore it. Not hacking around it.
FWIW, I just recently documented:
+/**
+ * pfnmap_setup_cachemode - setup the cachemode in the pgprot for a pfn range
+ * @pfn: the start of the pfn range
+ * @size: the size of the pfn range in bytes
+ * @prot: the pgprot to modify
+ *
+ * Lookup the cachemode for the pfn range starting at @pfn with the size
+ * @size and store it in @prot, leaving other data in @prot unchanged.
+ *
+ * This allows for a hardware implementation to have fine-grained control of
+ * memory cache behavior at page level granularity. Without a hardware
+ * implementation, this function does nothing.
+ *
+ * Currently there is only one implementation for this - x86 Page Attribute
+ * Table (PAT). See Documentation/arch/x86/pat.rst for more details.
+ *
+ * This function can fail if the pfn range spans pfns that require differing
+ * cachemodes. If the pfn range was previously verified to have a single
+ * cachemode, it is sufficient to query only a single pfn. The assumption is
+ * that this is the case for drivers using the vmf_insert_pfn*() interface.
+ *
+ * Returns 0 on success and -EINVAL on error.
+ */
+int pfnmap_setup_cachemode(unsigned long pfn, unsigned long size,
+ pgprot_t *prot);
extern int track_pfn_copy(struct vm_area_struct *dst_vma,
struct vm_area_struct *src_vma, unsigned long *pfn);
extern void untrack_pfn_copy(struct vm_area_struct *dst_vma,
@@ -1563,6 +1584,21 @@ extern void untrack_pfn(struct vm_area_struct *vma,
unsigned long pfn,
extern void untrack_pfn_clear(struct vm_area_struct *vma);
#endif
+/**
+ * pfnmap_setup_cachemode_pfn - setup the cachemode in the pgprot for a pfn
+ * @pfn: the pfn
+ * @prot: the pgprot to modify
+ *
+ * Lookup the cachemode for @pfn and store it in @prot, leaving other
+ * data in @prot unchanged.
+ *
+ * See pfnmap_setup_cachemode() for details.
+ */
+static inline void pfnmap_setup_cachemode_pfn(unsigned long pfn, pgprot_t
*prot)
+{
+ pfnmap_setup_cachemode(pfn, PAGE_SIZE, prot);
+}
There is certainly something missing that the driver would have previously
verified that the cachemode is as expected.
--
Cheers
David / dhildenb
