RE: [PATCH] x86/hyperv: Enable 15-bit APIC ID if the hypervisor supports it

2020-11-03 Thread Dexuan Cui
> From: David Woodhouse 
> Sent: Tuesday, November 3, 2020 12:03 AM
> > +/*
> > + * If ms_hyperv_msi_ext_dest_id() returns true,
> > hyperv_prepare_irq_remapping()
> > + * returns -ENODEV and the Hyper-V IOMMU driver is not used; instead, the
> > + * generic support of the 15-bit APIC ID is used: see
> > __irq_msi_compose_msg().
> > + *
> > + * Note: For a VM on Hyper-V, no emulated legacy device supports PCI
> MSI/MSI-X,
> > + * and PCI MSI/MSI-X only come from the assigned physical PCIe device, and
> the
> > + * PCI MSI/MSI-X interrupts are handled by the pci-hyperv driver. Here
> despite
> > + * the word "msi" in the name "msi_ext_dest_id", actually the callback only
> > + * affects how IOAPIC interrupts are routed.
> > + */
> 
> I named it like that on purpose to make the point that the I/OAPIC is
> just a device for turning line interrupts into MSIs. Some VMMs, just
> like real hardware, really do implement their I/OAPIC emulation that
> way. It makes a lot of sense to do so if you support interrupt
> remapping.

I totally agree.
 
> FWIW I might have phrased your last paragraph in that comment as
> 
>   Note: for a VM on Hyper-V, the I/OAPIC is the only device which
>   (logically) generates MSIs directly to the system APIC irq domain.
>   There is no HPET, and PCI MSI/MSI-X interrupts are remapped by the
>   pci-hyperv host bridge.

I agree. This version is much better.
 
> But don't bother to change it; I think I've made my point quite well
> enough with https://git.kernel.org/tip/tip/c/5d5a97133 :)
> 
> --
> dwmw2

Hi David,
This patch has been in the x86/apic branch (with a line missing in the commit
log). If possible, I hope tglx can help make this change you suggested, and add
the missing line in the commit log. :-)

Thanks,
-- Dexuan


RE: [PATCH] x86/hyperv: Enable 15-bit APIC ID if the hypervisor supports it

2020-11-03 Thread Dexuan Cui
> From: Dexuan Cui 
> Sent: Monday, November 2, 2020 5:12 PM
> 
> ...

Hi tglx,
Now this patch is in the x86/apic branch, which is great! Thanks for the 
quick action! But the third line of the below paragraph of the commit log
is missing... Sorry I just realized I should have not prefixed that line with 
the
">255 APIC IDs" -- it looks a line is ignored if it starts with 2 chars of 
">>". :-(

> On an old Hyper-V host that doesn't support the Extended Dest ID, nothing
> changes with this commit: Linux VM is still able to bring up the CPUs with
> >255 APIC IDs with the help of hyperv-iommu.c, but IOAPIC interrupts still
> can not go to such CPUs, and the kdump kernel still can not work properly
> on such CPUs.


Re: [PATCH] x86/hyperv: Enable 15-bit APIC ID if the hypervisor supports it

2020-11-03 Thread David Woodhouse
On Mon, 2020-11-02 at 17:11 -0800, Dexuan Cui wrote:
> When a Linux VM runs on Hyper-V, if the VM has CPUs with >255 APIC IDs,
> the CPUs can't be the destination of IOAPIC interrupts, because the
> IOAPIC RTE's Dest Field has only 8 bits. Currently the hackery driver
> drivers/iommu/hyperv-iommu.c is used to ensure IOAPIC interrupts are
> only routed to CPUs that don't have >255 APIC IDs. However, there is
> an issue with kdump, because the kdump kernel can run on any CPU, and
> hence IOAPIC interrupts can't work if the kdump kernel run on a CPU
> with a >255 APIC ID.
> 
> The kdump issue can be fixed by the Extended Dest ID, which is introduced
> recently by David Woodhouse (for IOAPIC, see the field virt_destid_8_14 in
> struct IO_APIC_route_entry). Of course, the Extended Dest ID needs the
> support of the underlying hypervisor. The latest Hyper-V has added the
> support recently: with this commit, on such a Hyper-V host, Linux VM
> does not use hyperv-iommu.c because hyperv_prepare_irq_remapping()
> returns -ENODEV; instead, Linux kernel's generic support of Extended Dest
> ID from David is used, meaning that Linux VM is able to support up to
> 32K CPUs, and IOAPIC interrupts can be routed to all the CPUs.
> 
> On an old Hyper-V host that doesn't support the Extended Dest ID, nothing
> changes with this commit: Linux VM is still able to bring up the CPUs with
> > 255 APIC IDs with the help of hyperv-iommu.c, but IOAPIC interrupts still
> 
> can not go to such CPUs, and the kdump kernel still can not work properly
> on such CPUs.
> 
> Signed-off-by: Dexuan Cui 

Acked-by: David Woodhouse 

> +/*
> + * If ms_hyperv_msi_ext_dest_id() returns true, 
> hyperv_prepare_irq_remapping()
> + * returns -ENODEV and the Hyper-V IOMMU driver is not used; instead, the
> + * generic support of the 15-bit APIC ID is used: see 
> __irq_msi_compose_msg().
> + *
> + * Note: For a VM on Hyper-V, no emulated legacy device supports PCI 
> MSI/MSI-X,
> + * and PCI MSI/MSI-X only come from the assigned physical PCIe device, and 
> the
> + * PCI MSI/MSI-X interrupts are handled by the pci-hyperv driver. Here 
> despite
> + * the word "msi" in the name "msi_ext_dest_id", actually the callback only
> + * affects how IOAPIC interrupts are routed.
> + */

I named it like that on purpose to make the point that the I/OAPIC is
just a device for turning line interrupts into MSIs. Some VMMs, just
like real hardware, really do implement their I/OAPIC emulation that
way. It makes a lot of sense to do so if you support interrupt
remapping.

FWIW I might have phrased your last paragraph in that comment as

  Note: for a VM on Hyper-V, the I/OAPIC is the only device which
  (logically) generates MSIs directly to the system APIC irq domain.
  There is no HPET, and PCI MSI/MSI-X interrupts are remapped by the
  pci-hyperv host bridge.

But don't bother to change it; I think I've made my point quite well
enough with https://git.kernel.org/tip/tip/c/5d5a97133 :)

-- 
dwmw2




smime.p7s
Description: S/MIME cryptographic signature


Re: [PATCH] x86/hyperv: Enable 15-bit APIC ID if the hypervisor supports it

2020-11-02 Thread kernel test robot
Hi Dexuan,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tip/x86/core]
[also build test ERROR on tip/master v5.10-rc2 next-20201102]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:
https://github.com/0day-ci/linux/commits/Dexuan-Cui/x86-hyperv-Enable-15-bit-APIC-ID-if-the-hypervisor-supports-it/20201103-091414
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 
238c91115cd05c71447ea071624a4c9fe661f970
config: i386-randconfig-r012-20201103 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
# 
https://github.com/0day-ci/linux/commit/c4037b8c4cd61f970749c6685a3df5a1376193d2
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review 
Dexuan-Cui/x86-hyperv-Enable-15-bit-APIC-ID-if-the-hypervisor-supports-it/20201103-091414
git checkout c4037b8c4cd61f970749c6685a3df5a1376193d2
# save the attached .config to linux build tree
make W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All error/warnings (new ones prefixed by >>):

>> arch/x86/kernel/cpu/mshyperv.c:397:8: error: 'struct x86_hyper_init' has no 
>> member named 'msi_ext_dest_id'
 397 |  .init.msi_ext_dest_id = ms_hyperv_msi_ext_dest_id,
 |^~~
>> arch/x86/kernel/cpu/mshyperv.c:397:26: error: initialization of 'void 
>> (*)(void)' from incompatible pointer type 'bool (*)(void)' {aka '_Bool 
>> (*)(void)'} [-Werror=incompatible-pointer-types]
 397 |  .init.msi_ext_dest_id = ms_hyperv_msi_ext_dest_id,
 |  ^
   arch/x86/kernel/cpu/mshyperv.c:397:26: note: (near initialization for 
'x86_hyper_ms_hyperv.init.init_platform')
>> arch/x86/kernel/cpu/mshyperv.c:398:24: warning: initialized field 
>> overwritten [-Woverride-init]
 398 |  .init.init_platform = ms_hyperv_init_platform,
 |^~~
   arch/x86/kernel/cpu/mshyperv.c:398:24: note: (near initialization for 
'x86_hyper_ms_hyperv.init.init_platform')
   cc1: some warnings being treated as errors

vim +397 arch/x86/kernel/cpu/mshyperv.c

   391  
   392  const __initconst struct hypervisor_x86 x86_hyper_ms_hyperv = {
   393  .name   = "Microsoft Hyper-V",
   394  .detect = ms_hyperv_platform,
   395  .type   = X86_HYPER_MS_HYPERV,
   396  .init.x2apic_available  = ms_hyperv_x2apic_available,
 > 397  .init.msi_ext_dest_id   = ms_hyperv_msi_ext_dest_id,
 > 398  .init.init_platform = ms_hyperv_init_platform,

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org


.config.gz
Description: application/gzip


RE: [PATCH] x86/hyperv: Enable 15-bit APIC ID if the hypervisor supports it

2020-11-02 Thread Dexuan Cui
> From: Dexuan Cui 
> Sent: Monday, November 2, 2020 5:12 PM

Sorry I forgot to mention that this patch is based on tip.git's x86/apic branch.

Thanks,
-- Dexuan


[PATCH] x86/hyperv: Enable 15-bit APIC ID if the hypervisor supports it

2020-11-02 Thread Dexuan Cui
When a Linux VM runs on Hyper-V, if the VM has CPUs with >255 APIC IDs,
the CPUs can't be the destination of IOAPIC interrupts, because the
IOAPIC RTE's Dest Field has only 8 bits. Currently the hackery driver
drivers/iommu/hyperv-iommu.c is used to ensure IOAPIC interrupts are
only routed to CPUs that don't have >255 APIC IDs. However, there is
an issue with kdump, because the kdump kernel can run on any CPU, and
hence IOAPIC interrupts can't work if the kdump kernel run on a CPU
with a >255 APIC ID.

The kdump issue can be fixed by the Extended Dest ID, which is introduced
recently by David Woodhouse (for IOAPIC, see the field virt_destid_8_14 in
struct IO_APIC_route_entry). Of course, the Extended Dest ID needs the
support of the underlying hypervisor. The latest Hyper-V has added the
support recently: with this commit, on such a Hyper-V host, Linux VM
does not use hyperv-iommu.c because hyperv_prepare_irq_remapping()
returns -ENODEV; instead, Linux kernel's generic support of Extended Dest
ID from David is used, meaning that Linux VM is able to support up to
32K CPUs, and IOAPIC interrupts can be routed to all the CPUs.

On an old Hyper-V host that doesn't support the Extended Dest ID, nothing
changes with this commit: Linux VM is still able to bring up the CPUs with
>255 APIC IDs with the help of hyperv-iommu.c, but IOAPIC interrupts still
can not go to such CPUs, and the kdump kernel still can not work properly
on such CPUs.

Signed-off-by: Dexuan Cui 
---
 arch/x86/include/asm/hyperv-tlfs.h |  7 +++
 arch/x86/kernel/cpu/mshyperv.c | 30 ++
 2 files changed, 37 insertions(+)

diff --git a/arch/x86/include/asm/hyperv-tlfs.h 
b/arch/x86/include/asm/hyperv-tlfs.h
index 0ed20e8bba9e..6bf42aed387e 100644
--- a/arch/x86/include/asm/hyperv-tlfs.h
+++ b/arch/x86/include/asm/hyperv-tlfs.h
@@ -23,6 +23,13 @@
 #define HYPERV_CPUID_IMPLEMENT_LIMITS  0x4005
 #define HYPERV_CPUID_NESTED_FEATURES   0x400A
 
+#define HYPERV_CPUID_VIRT_STACK_INTERFACE  0x4081
+#define HYPERV_VS_INTERFACE_EAX_SIGNATURE  0x31235356  /* "VS#1" */
+
+#define HYPERV_CPUID_VIRT_STACK_PROPERTIES 0x4082
+/* Support for the extended IOAPIC RTE format */
+#define HYPERV_VS_PROPERTIES_EAX_EXTENDED_IOAPIC_RTE   BIT(2)
+
 #define HYPERV_HYPERVISOR_PRESENT_BIT  0x8000
 #define HYPERV_CPUID_MIN   0x4005
 #define HYPERV_CPUID_MAX   0x4000
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index 05ef1f4550cb..cc4037d841df 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -366,9 +366,39 @@ static void __init ms_hyperv_init_platform(void)
 #endif
 }
 
+static bool __init ms_hyperv_x2apic_available(void)
+{
+   return x2apic_supported();
+}
+
+/*
+ * If ms_hyperv_msi_ext_dest_id() returns true, hyperv_prepare_irq_remapping()
+ * returns -ENODEV and the Hyper-V IOMMU driver is not used; instead, the
+ * generic support of the 15-bit APIC ID is used: see __irq_msi_compose_msg().
+ *
+ * Note: For a VM on Hyper-V, no emulated legacy device supports PCI MSI/MSI-X,
+ * and PCI MSI/MSI-X only come from the assigned physical PCIe device, and the
+ * PCI MSI/MSI-X interrupts are handled by the pci-hyperv driver. Here despite
+ * the word "msi" in the name "msi_ext_dest_id", actually the callback only
+ * affects how IOAPIC interrupts are routed.
+ */
+static bool __init ms_hyperv_msi_ext_dest_id(void)
+{
+   u32 eax;
+
+   eax = cpuid_eax(HYPERV_CPUID_VIRT_STACK_INTERFACE);
+   if (eax != HYPERV_VS_INTERFACE_EAX_SIGNATURE)
+   return false;
+
+   eax = cpuid_eax(HYPERV_CPUID_VIRT_STACK_PROPERTIES);
+   return eax & HYPERV_VS_PROPERTIES_EAX_EXTENDED_IOAPIC_RTE;
+}
+
 const __initconst struct hypervisor_x86 x86_hyper_ms_hyperv = {
.name   = "Microsoft Hyper-V",
.detect = ms_hyperv_platform,
.type   = X86_HYPER_MS_HYPERV,
+   .init.x2apic_available  = ms_hyperv_x2apic_available,
+   .init.msi_ext_dest_id   = ms_hyperv_msi_ext_dest_id,
.init.init_platform = ms_hyperv_init_platform,
 };
-- 
2.25.1