Hi Luca,
On 23/05/2023 08:43, Luca Fancellu wrote:
+int sve_context_init(struct vcpu *v)
+{
+ unsigned int sve_vl_bits = sve_decode_vl(v->domain->arch.sve_vl);
+ uint64_t *ctx = _xzalloc(sve_zreg_ctx_size(sve_vl_bits) +
+ sve_ffrreg_ctx_size(sve_vl_bits),
+ L1_CACHE_BYTES);
+
+ if ( !ctx )
+ return -ENOMEM;
+
+ /*
+ * Point to the end of Z0-Z31 memory, just before FFR memory, to be kept in
+ * sync with sve_context_free()
Nit: Missing a full stop.
+ */
+ v->arch.vfp.sve_zreg_ctx_end = ctx +
+ (sve_zreg_ctx_size(sve_vl_bits) / sizeof(uint64_t));
+
+ v->arch.zcr_el2 = vl_to_zcr(sve_vl_bits);
+
+ return 0;
+}
+
+void sve_context_free(struct vcpu *v)
+{
+ unsigned int sve_vl_bits;
+
+ if ( v->arch.vfp.sve_zreg_ctx_end )
+ return;
+
+ sve_vl_bits = sve_decode_vl(v->domain->arch.sve_vl);
+
+ /*
+ * Point to the end of Z0-Z31 memory, just before FFR memory, to be kept
+ * in sync with sve_context_init()
+ */
The spacing looks a bit odd in this comment. Did you miss an extra space?
Also, I notice this comment is the exact same as on top as
sve_context_init(). I think this is a bit misleading because the logic
is different. I would suggest the following:
"Currently points to the end of Z0-Z31 memory which is not the start of
the buffer. To be kept in sync with the sve_context_init()."
Lastly, nit: Missing a full stop.
+ v->arch.vfp.sve_zreg_ctx_end -=
+ (sve_zreg_ctx_size(sve_vl_bits) / sizeof(uint64_t));
+
+ XFREE(v->arch.vfp.sve_zreg_ctx_end);
+}
+
[...]
diff --git a/xen/arch/arm/include/asm/arm64/vfp.h
b/xen/arch/arm/include/asm/arm64/vfp.h
index e6e8c363bc16..4aa371e85d26 100644
--- a/xen/arch/arm/include/asm/arm64/vfp.h
+++ b/xen/arch/arm/include/asm/arm64/vfp.h
@@ -6,7 +6,19 @@
struct vfp_state
{
+ /*
+ * When SVE is enabled for the guest, fpregs memory will be used to
+ * save/restore P0-P15 registers, otherwise it will be used for the V0-V31
+ * registers.
+ */
uint64_t fpregs[64] __vfp_aligned;
+ /*
+ * When SVE is enabled for the guest, sve_zreg_ctx_end points to memory
+ * where Z0-Z31 registers and FFR can be saved/restored, it points at the
+ * end of the Z0-Z31 space and at the beginning of the FFR space, it's done
+ * like that to ease the save/restore assembly operations.
+ */
+ uint64_t *sve_zreg_ctx_end;
Sorry I only noticed now. But shouldn't this be protected with #ifdef
CONFIG_SVE? Same...
register_t fpcr;
register_t fpexc32_el2;
register_t fpsr;
diff --git a/xen/arch/arm/include/asm/domain.h
b/xen/arch/arm/include/asm/domain.h
index 331da0f3bcc3..814652d92568 100644
--- a/xen/arch/arm/include/asm/domain.h
+++ b/xen/arch/arm/include/asm/domain.h
@@ -195,6 +195,8 @@ struct arch_vcpu
register_t tpidrro_el0;
/* HYP configuration */
+ register_t zcr_el1;
+ register_t zcr_el2;
... here.
register_t cptr_el2;
register_t hcr_el2;
register_t mdcr_el2;
Cheers,
--
Julien Grall
