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

[Paul Durrant]

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