On Wed, Jan 27, 2021 at 05:47:04AM +0000, Michael Kelley wrote:
> From: Wei Liu <wei.liu@xxxxxxxxxx> Sent: Wednesday, January 20, 2021 4:01 AM
> >
> > When Linux runs as the root partition on Microsoft Hypervisor, its
> > interrupts are remapped. Linux will need to explicitly map and unmap
> > interrupts for hardware.
> >
> > Implement an MSI domain to issue the correct hypercalls. And initialize
> > this irqdomain as the default MSI irq domain.
> >
> > Signed-off-by: Sunil Muthuswamy <sunilmut@xxxxxxxxxxxxx>
> > Co-Developed-by: Sunil Muthuswamy <sunilmut@xxxxxxxxxxxxx>
> > Signed-off-by: Wei Liu <wei.liu@xxxxxxxxxx>
> > ---
> > v4: Fix compilation issue when CONFIG_PCI_MSI is not set.
> > v3: build irqdomain.o for 32bit as well.
>
> I'm not clear on the intent for 32-bit builds. Given that hv_proc.c is built
> only for 64-bit, I'm assuming running Linux in the root partition
> is only functional for 64-bit builds. So is the goal simply that 32-bit
> builds will compile correctly? Seems like maybe there should be
> a CONFIG option for running Linux in the root partition, and that
> option would force 64-bit.
To ensure 32 bit kernel builds and 32 bit guests still work.
The config option ROOT_API is to be introduced by Nuno's /dev/mshv
series. We can use that option to gate some objects when that's
available.
>
[...]
> > diff --git a/arch/x86/hyperv/irqdomain.c b/arch/x86/hyperv/irqdomain.c
> > new file mode 100644
> > index 000000000000..19637cd60231
> > --- /dev/null
> > +++ b/arch/x86/hyperv/irqdomain.c
> > @@ -0,0 +1,332 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +//
> > +// Irqdomain for Linux to run as the root partition on Microsoft Hypervisor.
> > +//
> > +// Authors:
> > +// Sunil Muthuswamy <sunilmut@xxxxxxxxxxxxx>
> > +// Wei Liu <wei.liu@xxxxxxxxxx>
>
> I think the // comment style should only be used for the SPDX line.
Fixed.
>
> > +
> > +#include <linux/pci.h>
> > +#include <linux/irq.h>
> > +#include <asm/mshyperv.h>
> > +
[...]
> > +static int hv_map_msi_interrupt(struct pci_dev *dev, int vcpu, 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;
> > + u16 status;
> > +
> > + 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 = hv_build_pci_dev_id(dev).as_uint64;
> > + intr_desc->interrupt_type = HV_X64_INTERRUPT_TYPE_FIXED;
> > + intr_desc->trigger_mode = HV_INTERRUPT_TRIGGER_MODE_EDGE;
> > + intr_desc->vector_count = 1;
> > + intr_desc->target.vector = vector;
> > + __set_bit(vcpu, (unsigned long*)&intr_desc->target.vp_mask);
>
> This is using the CPU bitmap format that supports up to 64 vCPUs. Any reason not
> to use the format that supports a larger number of CPUs? In either case, perhaps
> a check for the value of vcpu against the max of 64 (or the larger number if you
> change the bitmap format) would be appropriate.
>
This is mostly due to we didn't have a suitably large machine during
development.
I will see if this can use vpset instead.
> > +
> > + status = hv_do_rep_hypercall(HVCALL_MAP_DEVICE_INTERRUPT, 0, 0, input, output) &
> > + HV_HYPERCALL_RESULT_MASK;
> > + *entry = output->interrupt_entry;
> > +
> > + local_irq_restore(flags);
> > +
> > + if (status != HV_STATUS_SUCCESS)
> > + pr_err("%s: hypercall failed, status %d\n", __func__, status);
> > +
> > + return status;
> > +}
> > +
[...]
> > +static int hv_unmap_msi_interrupt(struct pci_dev *dev, struct hv_interrupt_entry
> > *old_entry)
> > +{
> > + return hv_unmap_interrupt(hv_build_pci_dev_id(dev).as_uint64, old_entry)
> > + & HV_HYPERCALL_RESULT_MASK;
>
> The masking with HV_HYPERCALL_RESULT_MASK is already done in
> hv_unmap_interrupt().
>
Fixed.
Wei.
