Michael Kelley
2021-Feb-04 18:40 UTC
[PATCH v6 15/16] x86/hyperv: implement an MSI domain for root partition
From: Wei Liu <wei.liu at kernel.org> Sent: Thursday, February 4, 2021 9:57 AM> > On Thu, Feb 04, 2021 at 05:43:16PM +0000, Michael Kelley wrote: > [...] > > > remove_cpuhp_state: > > > diff --git a/arch/x86/hyperv/irqdomain.c b/arch/x86/hyperv/irqdomain.c > > > new file mode 100644 > > > index 000000000000..117f17e8c88a > > > --- /dev/null > > > +++ b/arch/x86/hyperv/irqdomain.c > > > @@ -0,0 +1,362 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > + > > > +/* > > > + * for Linux to run as the root partition on Microsoft Hypervisor. > > > > Nit: Looks like the initial word "Irqdomain" got dropped from the above > > comment line. But don't respin just for this. > > > > I've added it back. Thanks. > > > > +static int hv_map_interrupt(union hv_device_id device_id, bool level, > > > + int cpu, int vector, struct hv_interrupt_entry *entry) > > > +{ > > > + struct hv_input_map_device_interrupt *input; > > > + struct hv_output_map_device_interrupt *output; > > > + struct hv_device_interrupt_descriptor *intr_desc; > > > + unsigned long flags; > > > + u64 status; > > > + cpumask_t mask = CPU_MASK_NONE; > > > + int nr_bank, var_size; > > > + > > > + local_irq_save(flags); > > > + > > > + input = *this_cpu_ptr(hyperv_pcpu_input_arg); > > > + output = *this_cpu_ptr(hyperv_pcpu_output_arg); > > > + > > > + intr_desc = &input->interrupt_descriptor; > > > + memset(input, 0, sizeof(*input)); > > > + input->partition_id = hv_current_partition_id; > > > + input->device_id = device_id.as_uint64; > > > + intr_desc->interrupt_type = HV_X64_INTERRUPT_TYPE_FIXED; > > > + intr_desc->vector_count = 1; > > > + intr_desc->target.vector = vector; > > > + > > > + if (level) > > > + intr_desc->trigger_mode = HV_INTERRUPT_TRIGGER_MODE_LEVEL; > > > + else > > > + intr_desc->trigger_mode = HV_INTERRUPT_TRIGGER_MODE_EDGE; > > > + > > > + cpumask_set_cpu(cpu, &mask); > > > + intr_desc->target.vp_set.valid_bank_mask = 0; > > > + intr_desc->target.vp_set.format = HV_GENERIC_SET_SPARSE_4K; > > > + nr_bank = cpumask_to_vpset(&(intr_desc->target.vp_set), &mask); > > > > There's a function get_cpu_mask() that returns a pointer to a cpumask with only > > the specified cpu set in the mask. It returns a const pointer to the correct entry > > in a pre-allocated array of all such cpumasks, so it's a lot more efficient than > > allocating and initializing a local cpumask instance on the stack. > > > > That's nice. > > I've got the following diff to fix both issues. If you're happy with the > changes, can you give your Reviewed-by? That saves a round of posting. > > diff --git a/arch/x86/hyperv/irqdomain.c b/arch/x86/hyperv/irqdomain.c > index 0cabc9aece38..fa71db798465 100644 > --- a/arch/x86/hyperv/irqdomain.c > +++ b/arch/x86/hyperv/irqdomain.c > @@ -1,7 +1,7 @@ > // SPDX-License-Identifier: GPL-2.0 > > /* > - * for Linux to run as the root partition on Microsoft Hypervisor. > + * Irqdomain for Linux to run as the root partition on Microsoft Hypervisor. > * > * Authors: > * Sunil Muthuswamy <sunilmut at microsoft.com> > @@ -20,7 +20,7 @@ static int hv_map_interrupt(union hv_device_id device_id, bool level, > struct hv_device_interrupt_descriptor *intr_desc; > unsigned long flags; > u64 status; > - cpumask_t mask = CPU_MASK_NONE; > + const cpumask_t *mask; > int nr_bank, var_size; > > local_irq_save(flags); > @@ -41,10 +41,10 @@ static int hv_map_interrupt(union hv_device_id device_id, bool > level, > else > intr_desc->trigger_mode = HV_INTERRUPT_TRIGGER_MODE_EDGE; > > - cpumask_set_cpu(cpu, &mask); > + mask = cpumask_of(cpu); > intr_desc->target.vp_set.valid_bank_mask = 0; > intr_desc->target.vp_set.format = HV_GENERIC_SET_SPARSE_4K; > - nr_bank = cpumask_to_vpset(&(intr_desc->target.vp_set), &mask); > + nr_bank = cpumask_to_vpset(&(intr_desc->target.vp_set), mask);Can you just do the following and get rid of the 'mask' local entirely? nr_bank = cpumask_to_vpset(&(intr_desc->target.vp_set), cpumask_of(cpu)); Either way, Reviewed-by: Michael Kelley <mikelley at microsoft.com>> if (nr_bank < 0) { > local_irq_restore(flags); > pr_err("%s: unable to generate VP set\n", __func__);