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