[PATCH v2 4/4] xen/riscv: init_csr_masks()-related improvements

Oleksii Kurochko posted 4 patches 1 day, 4 hours ago
[PATCH v2 4/4] xen/riscv: init_csr_masks()-related improvements
Posted by Oleksii Kurochko 1 day, 4 hours ago
There is no reason to use _UL() in define-s sitting in C file hence use UL
suffix instead.

Drop 3d argument of INIT_CSR_MASK() and INIT_RO_ONE_MASK() to reduce risk
of incomplete editing after copy-and-paste, or other typo-ing.

Use _VALID_ infix instead of _AVAIL_ as the mask identifies architecturally
defined bits, not bits available for software use.

Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
 xen/arch/riscv/domain.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/xen/arch/riscv/domain.c b/xen/arch/riscv/domain.c
index c327f44d07ca..c77be3b827eb 100644
--- a/xen/arch/riscv/domain.c
+++ b/xen/arch/riscv/domain.c
@@ -42,10 +42,10 @@ struct csr_masks {
 
 static struct csr_masks __ro_after_init csr_masks;
 
-#define HEDELEG_AVAIL_MASK ULONG_MAX
-#define HIDELEG_AVAIL_MASK ULONG_MAX
-#define HENVCFG_AVAIL_MASK _UL(0xE0000003000000FF)
-#define HSTATEEN0_AVAIL_MASK _UL(0xDE00000000000007)
+#define HEDELEG_VALID_MASK ULONG_MAX
+#define HIDELEG_VALID_MASK ULONG_MAX
+#define HENVCFG_VALID_MASK 0xe0000003000000ffUL
+#define HSTATEEN0_VALID_MASK 0xde00000000000007UL
 
 void __init init_csr_masks(void)
 {
@@ -57,25 +57,26 @@ void __init init_csr_masks(void)
      * fields that must be preserved. Any write to the full register must
      * therefore retain the original values of those fields.
      */
-#define INIT_CSR_MASK(csr, field, mask) do { \
-        register_t old = csr_read_set(CSR_ ## csr, mask); \
+#define INIT_CSR_MASK(csr, field) do { \
+        register_t old = csr_read_set(CSR_ ## csr, csr ## _VALID_MASK); \
         csr_masks.field = csr_swap(CSR_ ## csr, old); \
     } while (0)
 
-#define INIT_RO_ONE_MASK(csr, field, mask) do { \
-        register_t old = csr_read_clear(CSR_ ## csr, mask); \
-        csr_masks.ro_one.field = csr_swap(CSR_ ## csr, old) & mask; \
+#define INIT_RO_ONE_MASK(csr, field) do { \
+        register_t old = csr_read_clear(CSR_ ## csr, csr ## _VALID_MASK); \
+        csr_masks.ro_one.field = csr_swap(CSR_ ## csr, old) & \
+                                 csr ## _VALID_MASK; \
     } while (0)
 
-    INIT_CSR_MASK(HEDELEG, hedeleg, HEDELEG_AVAIL_MASK);
-    INIT_CSR_MASK(HIDELEG, hideleg, HIDELEG_AVAIL_MASK);
+    INIT_CSR_MASK(HEDELEG, hedeleg);
+    INIT_CSR_MASK(HIDELEG, hideleg);
 
-    INIT_CSR_MASK(HENVCFG, henvcfg, HENVCFG_AVAIL_MASK);
+    INIT_CSR_MASK(HENVCFG, henvcfg);
 
     if ( riscv_isa_extension_available(NULL, RISCV_ISA_EXT_smstateen) )
     {
-        INIT_CSR_MASK(HSTATEEN0, hstateen0, HSTATEEN0_AVAIL_MASK);
-        INIT_RO_ONE_MASK(HSTATEEN0, hstateen0, HSTATEEN0_AVAIL_MASK);
+        INIT_CSR_MASK(HSTATEEN0, hstateen0);
+        INIT_RO_ONE_MASK(HSTATEEN0, hstateen0);
     }
 
 #undef INIT_CSR_MASK
-- 
2.53.0
Re: [PATCH v2 4/4] xen/riscv: init_csr_masks()-related improvements
Posted by Jan Beulich 17 hours ago
On 31.03.2026 21:04, Oleksii Kurochko wrote:
> There is no reason to use _UL() in define-s sitting in C file hence use UL
> suffix instead.
> 
> Drop 3d argument of INIT_CSR_MASK() and INIT_RO_ONE_MASK() to reduce risk
> of incomplete editing after copy-and-paste, or other typo-ing.
> 
> Use _VALID_ infix instead of _AVAIL_ as the mask identifies architecturally
> defined bits, not bits available for software use.
> 
> Suggested-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Seeing this is ready to go in, am I overlooking any dependency on earlier
patches, or could this indeed go in right away?

Jan
Re: [PATCH v2 4/4] xen/riscv: init_csr_masks()-related improvements
Posted by Oleksii Kurochko 9 hours ago

On 4/1/26 8:19 AM, Jan Beulich wrote:
> On 31.03.2026 21:04, Oleksii Kurochko wrote:
>> There is no reason to use _UL() in define-s sitting in C file hence use UL
>> suffix instead.
>>
>> Drop 3d argument of INIT_CSR_MASK() and INIT_RO_ONE_MASK() to reduce risk
>> of incomplete editing after copy-and-paste, or other typo-ing.
>>
>> Use _VALID_ infix instead of _AVAIL_ as the mask identifies architecturally
>> defined bits, not bits available for software use.
>>
>> Suggested-by: Jan Beulich <jbeulich@suse.com>
>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 
> Seeing this is ready to go in, am I overlooking any dependency on earlier
> patches, or could this indeed go in right away?

No, there is no any dependency, it could go earlier then other patches 
of this patch series.

Thanks.

~ Oleksii