Re: [PATCH v3 01/20] xen/riscv: implement sbi_remote_hfence_gvma()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 31.07.2025 17:58, Oleksii Kurochko wrote:
> Instruct the remote harts to execute one or more HFENCE.GVMA instructions,
> covering the range of guest physical addresses between start_addr and
> start_addr + size for all VMIDs.
> 
> The remote fence operation applies to the entire address space if either:
>  - start_addr and size are both 0, or
>  - size is equal to 2^XLEN-1.
> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>

Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

However, ...

> --- a/xen/arch/riscv/include/asm/sbi.h
> +++ b/xen/arch/riscv/include/asm/sbi.h
> @@ -89,6 +89,25 @@ bool sbi_has_rfence(void);
>  int sbi_remote_sfence_vma(const cpumask_t *cpu_mask, vaddr_t start,
>                            size_t size);
>  
> +/*
> + * Instructs the remote harts to execute one or more HFENCE.GVMA
> + * instructions, covering the range of guest physical addresses
> + * between start_addr and start_addr + size for all VMIDs.

... I'd like to ask that you avoid fuzzy terminology like this one. Afaict
you mean [start, start + size). Help yourself and future readers by then
also saying it exactly like this. (Happy to make a respective edit while
committing.)

> + * Returns 0 if IPI was sent to all the targeted harts successfully
> + * or negative value if start_addr or size is not valid.

This similarly is ambiguous: The union of the success case stated and the
error case stated isn't obviously all possible states. The success
statement in particular alludes to the possibility of an IPI not actually
reaching its target.

> + * The remote fence operation applies to the entire address space if either:
> + *  - start_addr and size are both 0, or
> + *  - size is equal to 2^XLEN-1.

Whose XLEN is this? The guest's? The host's? (I assume the latter, but it's
not unambiguous, unless there's specific terminology that I'm unaware of,
yet which would make this unambiguous.)

Jan