Introduce set_fixmap() and clear_fixmap() functions to manage mappings
in the fixmap region. The set_fixmap() function maps a 4k page ( as only L0
is expected to be updated; look at setup_fixmap_mappings() ) at a specified
fixmap entry using map_pages_to_xen(), while clear_fixmap() removes the
mapping from a fixmap entry by calling destroy_xen_mappings().

Both functions ensure that the operations succeed by asserting that their
respective calls (map_pages_to_xen() and destroy_xen_mappings()) return 0.
A `BUG_ON` check is used to trigger a failure if any issues occur during
the mapping or unmapping process.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
 xen/arch/riscv/include/asm/fixmap.h |  5 +++++
 xen/arch/riscv/pt.c                 | 19 +++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/xen/arch/riscv/include/asm/fixmap.h b/xen/arch/riscv/include/asm/fixmap.h
index 818c8ce07b..e399a15f53 100644
--- a/xen/arch/riscv/include/asm/fixmap.h
+++ b/xen/arch/riscv/include/asm/fixmap.h
@@ -32,6 +32,11 @@
  */
 extern pte_t xen_fixmap[];
 
+/* Map a page in a fixmap entry */
+void set_fixmap(unsigned int map, mfn_t mfn, unsigned int flags);
+/* Remove a mapping from a fixmap entry */
+void clear_fixmap(unsigned int map);
+
 #define fix_to_virt(slot) ((void *)FIXMAP_ADDR(slot))
 
 static inline unsigned int virt_to_fix(vaddr_t vaddr)
diff --git a/xen/arch/riscv/pt.c b/xen/arch/riscv/pt.c
index 8d35ef5ca8..ed9a943d4c 100644
--- a/xen/arch/riscv/pt.c
+++ b/xen/arch/riscv/pt.c
@@ -8,6 +8,7 @@
 #include <xen/pmap.h>
 #include <xen/spinlock.h>
 
+#include <asm/fixmap.h>
 #include <asm/flushtlb.h>
 #include <asm/page.h>
 
@@ -433,3 +434,21 @@ int __init populate_pt_range(unsigned long virt, unsigned long nr_mfns)
 {
     return pt_update(virt, INVALID_MFN, nr_mfns, PTE_POPULATE);
 }
+
+/* Map a 4k page in a fixmap entry */
+void set_fixmap(unsigned int map, mfn_t mfn, unsigned int flags)
+{
+    int res;
+
+    res = map_pages_to_xen(FIXMAP_ADDR(map), mfn, 1, flags | PTE_SMALL);
+    BUG_ON(res != 0);
+}
+
+/* Remove a mapping from a fixmap entry */
+void clear_fixmap(unsigned int map)
+{
+    int res;
+
+    res = destroy_xen_mappings(FIXMAP_ADDR(map), FIXMAP_ADDR(map) + PAGE_SIZE);
+    BUG_ON(res != 0);
+}
-- 
2.47.0

On 27.11.2024 13:50, Oleksii Kurochko wrote:
> Introduce set_fixmap() and clear_fixmap() functions to manage mappings
> in the fixmap region. The set_fixmap() function maps a 4k page ( as only L0
> is expected to be updated; look at setup_fixmap_mappings() ) at a specified
> fixmap entry using map_pages_to_xen(), while clear_fixmap() removes the
> mapping from a fixmap entry by calling destroy_xen_mappings().
> 
> Both functions ensure that the operations succeed by asserting that their
> respective calls (map_pages_to_xen() and destroy_xen_mappings()) return 0.
> A `BUG_ON` check is used to trigger a failure if any issues occur during
> the mapping or unmapping process.
> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

However, ...

> @@ -433,3 +434,21 @@ int __init populate_pt_range(unsigned long virt, unsigned long nr_mfns)
>  {
>      return pt_update(virt, INVALID_MFN, nr_mfns, PTE_POPULATE);
>  }
> +
> +/* Map a 4k page in a fixmap entry */
> +void set_fixmap(unsigned int map, mfn_t mfn, unsigned int flags)
> +{
> +    int res;
> +
> +    res = map_pages_to_xen(FIXMAP_ADDR(map), mfn, 1, flags | PTE_SMALL);
> +    BUG_ON(res != 0);
> +}

... imo in such cases it is preferable to go without a local variable:

    if ( map_pages_to_xen(FIXMAP_ADDR(map), mfn, 1, flags | PTE_SMALL) != 0 )
        BUG();

Just to double check: Iirc this BUG would in particular trigger when trying
to set a fixmap slot that was already set, and not intermediately cleared?

Jan

On 12/9/24 3:29 PM, Jan Beulich wrote:
> On 27.11.2024 13:50, Oleksii Kurochko wrote:
>> Introduce set_fixmap() and clear_fixmap() functions to manage mappings
>> in the fixmap region. The set_fixmap() function maps a 4k page ( as only L0
>> is expected to be updated; look at setup_fixmap_mappings() ) at a specified
>> fixmap entry using map_pages_to_xen(), while clear_fixmap() removes the
>> mapping from a fixmap entry by calling destroy_xen_mappings().
>>
>> Both functions ensure that the operations succeed by asserting that their
>> respective calls (map_pages_to_xen() and destroy_xen_mappings()) return 0.
>> A `BUG_ON` check is used to trigger a failure if any issues occur during
>> the mapping or unmapping process.
>>
>> Signed-off-by: Oleksii Kurochko<oleksii.kurochko@gmail.com>
> Acked-by: Jan Beulich<jbeulich@suse.com>
>
> However, ...
>
>> @@ -433,3 +434,21 @@ int __init populate_pt_range(unsigned long virt, unsigned long nr_mfns)
>>   {
>>       return pt_update(virt, INVALID_MFN, nr_mfns, PTE_POPULATE);
>>   }
>> +
>> +/* Map a 4k page in a fixmap entry */
>> +void set_fixmap(unsigned int map, mfn_t mfn, unsigned int flags)
>> +{
>> +    int res;
>> +
>> +    res = map_pages_to_xen(FIXMAP_ADDR(map), mfn, 1, flags | PTE_SMALL);
>> +    BUG_ON(res != 0);
>> +}
> ... imo in such cases it is preferable to go without a local variable:
>
>      if ( map_pages_to_xen(FIXMAP_ADDR(map), mfn, 1, flags | PTE_SMALL) != 0 )
>          BUG();

I will update that in the next patch version.

>
> Just to double check: Iirc this BUG would in particular trigger when trying
> to set a fixmap slot that was already set, and not intermediately cleared?

Yes, correct.

Thanks.

~ Oleksii