Re: [PATCH v12 11/20] KVM: xen: allow shared_info to be mapped by fixed HVA

On 07/02/2024 04:10, Sean Christopherson wrote:

On Mon, Jan 15, 2024, Paul Durrant wrote:

...

@@ -638,20 +637,32 @@ int kvm_xen_hvm_set_attr(struct kvm *kvm, struct kvm_xen_hvm_attr *data) } break;

• case KVM_XEN_ATTR_TYPE_SHARED_INFO: {

• case KVM_XEN_ATTR_TYPE_SHARED_INFO:
• case KVM_XEN_ATTR_TYPE_SHARED_INFO_HVA: { int idx;

mutex_lock(&kvm->arch.xen.xen_lock); idx = srcu_read_lock(&kvm->srcu);

• if (data->u.shared_info.gfn == KVM_XEN_INVALID_GFN) {
  

• 	kvm_gpc_deactivate(&kvm->arch.xen.shinfo_cache);
  

• 	r = 0;
  

• if (data->type == KVM_XEN_ATTR_TYPE_SHARED_INFO) {
  

• 	if (data->u.shared_info.gfn == KVM_XEN_INVALID_GFN) {
  

• 		kvm_gpc_deactivate(&kvm->arch.xen.shinfo_cache);
  

• 		r = 0;
  

• 	} else {
  

• 		r = kvm_gpc_activate(&kvm->arch.xen.shinfo_cache,
  

• 				     gfn_to_gpa(data->u.shared_info.gfn),
  

• 				     PAGE_SIZE);
  

• 	}
  

  } else {

• 	r = kvm_gpc_activate(&kvm->arch.xen.shinfo_cache,
  

• 			     gfn_to_gpa(data->u.shared_info.gfn),
  

• 			     PAGE_SIZE);
  

• 	if (data->u.shared_info.hva == 0) {
  

I know I said I don't care about the KVM Xen ABI, but I still think using '0' as "invalid" is ridiculous.

With the benefit of some sleep, I'm wondering why 0 is a 'ridiculous' invalid value for a *virtual* address? Surely it's essentially a numerical cast of the canonically invalid NULL pointer?

Paul