RE: [RFC PATCH 06/18] virt/mshv: create, initialize, finalize, delete partition hypercalls

[Michael Kelley <mikelley@microsoft.com>]

From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Thursday, March 4, 2021 3:49 PM
> 
> On 2/8/2021 11:42 AM, Michael Kelley wrote:
> > From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Friday, November
> 20, 2020 4:30 PM
> >>

[snip]

> >> +
> >> +static int
> >> +hv_call_create_partition(
> >> +		u64 flags,
> >> +		struct hv_partition_creation_properties creation_properties,
> >> +		u64 *partition_id)
> >> +{
> >> +	struct hv_create_partition_in *input;
> >> +	struct hv_create_partition_out *output;
> >> +	int status;
> >> +	int ret;
> >> +	unsigned long irq_flags;
> >> +	int i;
> >> +
> >> +	do {
> >> +		local_irq_save(irq_flags);
> >> +		input = (struct hv_create_partition_in *)(*this_cpu_ptr(
> >> +			hyperv_pcpu_input_arg));
> >> +		output = (struct hv_create_partition_out *)(*this_cpu_ptr(
> >> +			hyperv_pcpu_output_arg));
> >> +
> >> +		input->flags = flags;
> >> +		input->proximity_domain_info.as_uint64 = 0;
> >> +		input->compatibility_version = HV_COMPATIBILITY_MANGANESE;
> >> +		for (i = 0; i < HV_PARTITION_PROCESSOR_FEATURE_BANKS; ++i)
> >> +			input->partition_creation_properties
> >> +				.disabled_processor_features.as_uint64[i] = 0;
> >> +		input->partition_creation_properties
> >> +			.disabled_processor_xsave_features.as_uint64 = 0;
> >> +		input->isolation_properties.as_uint64 = 0;
> >> +
> >> +		status = hv_do_hypercall(HVCALL_CREATE_PARTITION,
> >> +					 input, output);
> >
> > hv_do_hypercall returns a u64, which should then be masked with
> > HV_HYPERCALL_RESULT_MASK before checking the result.
> >
> 
> Yes, I'll fix this everywhere.
> 
> >> +		if (status != HV_STATUS_INSUFFICIENT_MEMORY) {
> >> +			if (status == HV_STATUS_SUCCESS)
> >> +				*partition_id = output->partition_id;
> >> +			else
> >> +				pr_err("%s: %s\n",
> >> +				       __func__, hv_status_to_string(status));
> >> +			local_irq_restore(irq_flags);
> >> +			ret = -hv_status_to_errno(status);
> >> +			break;
> >> +		}
> >> +		local_irq_restore(irq_flags);
> >> +		ret = hv_call_deposit_pages(NUMA_NO_NODE,
> >> +					    hv_current_partition_id, 1);
> >> +	} while (!ret);
> >> +
> >> +	return ret;
> >> +}
> >> +

I had a separate thread on the linux-hyperv mailing list about the
inconsistency in how we check hypercall status in current upstream
code, and proposed some helper functions to make it easier and
more consistent.  Joe Salisbury has started work on a patch to
provide those helper functions and to start using them in current
upstream code.  You could coordinate with Joe to get the helper
functions as well and use them as discussed in that thread.  Then
later on we won't have to come back and fix up the uses in this
patch series.

Michael