On 8/4/25 3:52 PM, Jan Beulich wrote:
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.kuroc...@gmail.com>
Acked-by: Jan Beulich<jbeul...@suse.com>
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.)
I just tried the following wording in SBI spec.
I agree that using [start, start+size) is clearer as each time I'm going
to check SBI code to verify if 'start+size' is included or not.
It would be happy if you could update this part of commit message during
commit.
+ * 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 same as above this is what SBI spec. tells.
I've not checked SBI code deeply, but it seems like the code is waiting while
IPI will be reached as looking at the code:
/**
* As this this function only handlers scalar values of hart mask, it
must be
* set to all online harts if the intention is to send IPIs to all the
harts.
* If hmask is zero, no IPIs will be sent.
*/
int sbi_ipi_send_many(ulong hmask, ulong hbase, u32 event, void *data)
{
...
/* Send IPIs */
do {
retry_needed = false;
sbi_hartmask_for_each_hart(i, &target_mask) {
rc = sbi_ipi_send(scratch, i, event, data);
if (rc == SBI_IPI_UPDATE_RETRY)
retry_needed = true;
else
sbi_hartmask_clear_hart(i,
&target_mask);
}
} while (retry_needed);
/* Sync IPIs */
sbi_ipi_sync(scratch, event);
return 0;
}
and
static int sbi_ipi_sync(struct sbi_scratch *scratch, u32 event)
{
const struct sbi_ipi_event_ops *ipi_ops;
if ((SBI_IPI_EVENT_MAX <= event) ||
!ipi_ops_array[event])
return SBI_EINVAL;
ipi_ops = ipi_ops_array[event];
if (ipi_ops->sync)
ipi_ops->sync(scratch);
return 0;
}
which calls:
static void tlb_sync(struct sbi_scratch *scratch)
{
atomic_t *tlb_sync =
sbi_scratch_offset_ptr(scratch, tlb_sync_off);
while (atomic_read(tlb_sync) > 0) {
/*
* While we are waiting for remote hart to set the sync,
* consume fifo requests to avoid deadlock.
*/
tlb_process_once(scratch);
}
return;
}
+ * 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.)
RISC-V spec quite mixes the terminology (3.1.6.2. Base ISA Control in mstatus
Register)
around XLEN:
For RV64 harts, the SXL and UXL fields are WARL fields that control the value
of XLEN for S-mode and U-mode, respectively. The encoding of these fields is
the same as the MXL field of misa, shown in Table 9. The effective XLEN in
S-mode and U-mode are termed SXLEN and UXLEN, respectively
Basically, RISC-V privileged architecture defines different XLEN values for
various privilege modes:
- MXLEN for Machine mode
- SXLEN for Supervisor mode.
- HSXLEN for Hypervisor-Supervisor mode.
- VSXLEN for Virtual Supervisor mode.
Considering that SBI is an API that is provided for S-mode I expect that XLEN =
SXLEN
in this case, but SBI spec. is using just XLEN.
~ Oleksii