[PATCH kvmtool 1/2] kvm tools: fix initialization of irq mptable

[PATCH kvmtool 1/2] kvm tools: fix initialization of irq mptable

[Muchun Song]

When dev_hdr->dev_num is greater one, the initialization of last_addr
is wrong.  Fix it.

Fixes: f83cd16 ("kvm tools: irq: replace the x86 irq rbtree with the PCI device tree")
Signed-off-by: Muchun Song <songmuchun@bytedance.com>
---
 x86/mptable.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/x86/mptable.c b/x86/mptable.c
index a984de9..f13cf0f 100644
--- a/x86/mptable.c
+++ b/x86/mptable.c
@@ -184,7 +184,7 @@ int mptable__init(struct kvm *kvm)
 		mpc_intsrc = last_addr;
 		mptable_add_irq_src(mpc_intsrc, pcibusid, srcbusirq, ioapicid, pci_hdr->irq_line);
 
-		last_addr = (void *)&mpc_intsrc[dev_hdr->dev_num];
+		last_addr = (void *)&mpc_intsrc[1];
 		nentries++;
 		dev_hdr = device__next_dev(dev_hdr);
 	}
-- 
2.11.0

[PATCH kvmtool 2/2] kvm tools: avoid printing [Firmware Bug]: CPUx: APIC id mismatch. Firmware: x APIC: x

[Muchun Song]

When I boot kernel, the dmesg will print the following message:

  [Firmware Bug]: CPU1: APIC id mismatch. Firmware: 1 APIC: 30

Fix this by setting up correct initial_apicid to cpu_id.

Signed-off-by: Muchun Song <songmuchun@bytedance.com>
---
 x86/cpuid.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/x86/cpuid.c b/x86/cpuid.c
index c3b67d9..aa213d5 100644
--- a/x86/cpuid.c
+++ b/x86/cpuid.c
@@ -8,7 +8,7 @@
 
 #define	MAX_KVM_CPUID_ENTRIES		100
 
-static void filter_cpuid(struct kvm_cpuid2 *kvm_cpuid)
+static void filter_cpuid(struct kvm_cpuid2 *kvm_cpuid, int cpu_id)
 {
 	unsigned int signature[3];
 	unsigned int i;
@@ -28,6 +28,8 @@ static void filter_cpuid(struct kvm_cpuid2 *kvm_cpuid)
 			entry->edx = signature[2];
 			break;
 		case 1:
+			entry->ebx &= ~(0xff << 24);
+			entry->ebx |= cpu_id << 24;
 			/* Set X86_FEATURE_HYPERVISOR */
 			if (entry->index == 0)
 				entry->ecx |= (1 << 31);
@@ -80,7 +82,7 @@ void kvm_cpu__setup_cpuid(struct kvm_cpu *vcpu)
 	if (ioctl(vcpu->kvm->sys_fd, KVM_GET_SUPPORTED_CPUID, kvm_cpuid) < 0)
 		die_perror("KVM_GET_SUPPORTED_CPUID failed");
 
-	filter_cpuid(kvm_cpuid);
+	filter_cpuid(kvm_cpuid, vcpu->cpu_id);
 
 	if (ioctl(vcpu->vcpu_fd, KVM_SET_CPUID2, kvm_cpuid) < 0)
 		die_perror("KVM_SET_CPUID2 failed");
-- 
2.11.0

Re: [PATCH kvmtool 2/2] kvm tools: avoid printing [Firmware Bug]: CPUx: APIC id mismatch. Firmware: x APIC: x

[Alexandru Elisei]

Hi,

Would you mind rewording the commit subject to:

x86: Set the correct APIC ID

On Wed, Feb 16, 2022 at 12:38:34PM +0800, Muchun Song wrote:
> When I boot kernel, the dmesg will print the following message:
  ^^^^^^^^^^^^^^^^^^

Would you mind replacing that with "When kvmtool boots a kernel, [..]"?

> 
>   [Firmware Bug]: CPU1: APIC id mismatch. Firmware: 1 APIC: 30
> 
> Fix this by setting up correct initial_apicid to cpu_id.

Thank you for fixing this. I've always wanted to fix that error, but I didn't
know enough about the x86 architecture.

> 
> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> ---
>  x86/cpuid.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/x86/cpuid.c b/x86/cpuid.c
> index c3b67d9..aa213d5 100644
> --- a/x86/cpuid.c
> +++ b/x86/cpuid.c
> @@ -8,7 +8,7 @@
>  
>  #define	MAX_KVM_CPUID_ENTRIES		100
>  
> -static void filter_cpuid(struct kvm_cpuid2 *kvm_cpuid)
> +static void filter_cpuid(struct kvm_cpuid2 *kvm_cpuid, int cpu_id)
>  {
>  	unsigned int signature[3];
>  	unsigned int i;
> @@ -28,6 +28,8 @@ static void filter_cpuid(struct kvm_cpuid2 *kvm_cpuid)
>  			entry->edx = signature[2];
>  			break;
>  		case 1:
> +			entry->ebx &= ~(0xff << 24);
> +			entry->ebx |= cpu_id << 24;
>  			/* Set X86_FEATURE_HYPERVISOR */
>  			if (entry->index == 0)
>  				entry->ecx |= (1 << 31);
> @@ -80,7 +82,7 @@ void kvm_cpu__setup_cpuid(struct kvm_cpu *vcpu)
>  	if (ioctl(vcpu->kvm->sys_fd, KVM_GET_SUPPORTED_CPUID, kvm_cpuid) < 0)
>  		die_perror("KVM_GET_SUPPORTED_CPUID failed");
>  
> -	filter_cpuid(kvm_cpuid);
> +	filter_cpuid(kvm_cpuid, vcpu->cpu_id);

Tested it and it works:

Tested-by: Alexandru Elisei <alexandru.elisei@arm.com>

Thanks,
Alex

>  
>  	if (ioctl(vcpu->vcpu_fd, KVM_SET_CPUID2, kvm_cpuid) < 0)
>  		die_perror("KVM_SET_CPUID2 failed");
> -- 
> 2.11.0
> 

Re: [PATCH kvmtool 1/2] kvm tools: fix initialization of irq mptable

[Alexandru Elisei]

Hi,

On Wed, Feb 16, 2022 at 12:38:33PM +0800, Muchun Song wrote:
> When dev_hdr->dev_num is greater one, the initialization of last_addr
> is wrong.  Fix it.
> 
> Fixes: f83cd16 ("kvm tools: irq: replace the x86 irq rbtree with the PCI device tree")
> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> ---
>  x86/mptable.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/x86/mptable.c b/x86/mptable.c
> index a984de9..f13cf0f 100644
> --- a/x86/mptable.c
> +++ b/x86/mptable.c
> @@ -184,7 +184,7 @@ int mptable__init(struct kvm *kvm)
>  		mpc_intsrc = last_addr;
>  		mptable_add_irq_src(mpc_intsrc, pcibusid, srcbusirq, ioapicid, pci_hdr->irq_line);
>  
> -		last_addr = (void *)&mpc_intsrc[dev_hdr->dev_num];
> +		last_addr = (void *)&mpc_intsrc[1];

This looks correct to me. I think there was a copy-and-paste error from the
device loop above where the interrupt lines were added to mpc_intsrc.

Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Thanks,
Alex

>  		nentries++;
>  		dev_hdr = device__next_dev(dev_hdr);
>  	}
> -- 
> 2.11.0
> 

Re: [PATCH kvmtool 1/2] kvm tools: fix initialization of irq mptable

[Alexandru Elisei]

Hi,

Forgot to add, would you mind rewording the commit subject to:

x86: Fix initialization of irq mptable

Thanks,
Alex

On Wed, Feb 16, 2022 at 10:59:36AM +0000, Alexandru Elisei wrote:
> Hi,
> 
> On Wed, Feb 16, 2022 at 12:38:33PM +0800, Muchun Song wrote:
> > When dev_hdr->dev_num is greater one, the initialization of last_addr
> > is wrong.  Fix it.
> > 
> > Fixes: f83cd16 ("kvm tools: irq: replace the x86 irq rbtree with the PCI device tree")
> > Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> > ---
> >  x86/mptable.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/x86/mptable.c b/x86/mptable.c
> > index a984de9..f13cf0f 100644
> > --- a/x86/mptable.c
> > +++ b/x86/mptable.c
> > @@ -184,7 +184,7 @@ int mptable__init(struct kvm *kvm)
> >  		mpc_intsrc = last_addr;
> >  		mptable_add_irq_src(mpc_intsrc, pcibusid, srcbusirq, ioapicid, pci_hdr->irq_line);
> >  
> > -		last_addr = (void *)&mpc_intsrc[dev_hdr->dev_num];
> > +		last_addr = (void *)&mpc_intsrc[1];
> 
> This looks correct to me. I think there was a copy-and-paste error from the
> device loop above where the interrupt lines were added to mpc_intsrc.
> 
> Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>
> 
> Thanks,
> Alex
> 
> >  		nentries++;
> >  		dev_hdr = device__next_dev(dev_hdr);
> >  	}
> > -- 
> > 2.11.0
> > 

Re: [External] Re: [PATCH kvmtool 1/2] kvm tools: fix initialization of irq mptable

[Muchun Song]

On Wed, Feb 16, 2022 at 7:00 PM Alexandru Elisei
<alexandru.elisei@arm.com> wrote:
>
> Hi,
>
> Forgot to add, would you mind rewording the commit subject to:
>
> x86: Fix initialization of irq mptable
>

Will do. Thanks.

Re: [PATCH kvmtool 2/2] kvm tools: avoid printing [Firmware Bug]: CPUx: APIC id mismatch. Firmware: x APIC: x

[Muchun Song]

On Wed, Feb 16, 2022 at 6:48 PM Alexandru Elisei
<alexandru.elisei@arm.com> wrote:
>
> Hi,
>
> Would you mind rewording the commit subject to:
>
> x86: Set the correct APIC ID
>
> On Wed, Feb 16, 2022 at 12:38:34PM +0800, Muchun Song wrote:
> > When I boot kernel, the dmesg will print the following message:
>   ^^^^^^^^^^^^^^^^^^
>
> Would you mind replacing that with "When kvmtool boots a kernel, [..]"?

Will do.

>
> >
> >   [Firmware Bug]: CPU1: APIC id mismatch. Firmware: 1 APIC: 30
> >
> > Fix this by setting up correct initial_apicid to cpu_id.
>
> Thank you for fixing this. I've always wanted to fix that error, but I didn't
> know enough about the x86 architecture.
>
> >
> > Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> > ---
> >  x86/cpuid.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/x86/cpuid.c b/x86/cpuid.c
> > index c3b67d9..aa213d5 100644
> > --- a/x86/cpuid.c
> > +++ b/x86/cpuid.c
> > @@ -8,7 +8,7 @@
> >
> >  #define      MAX_KVM_CPUID_ENTRIES           100
> >
> > -static void filter_cpuid(struct kvm_cpuid2 *kvm_cpuid)
> > +static void filter_cpuid(struct kvm_cpuid2 *kvm_cpuid, int cpu_id)
> >  {
> >       unsigned int signature[3];
> >       unsigned int i;
> > @@ -28,6 +28,8 @@ static void filter_cpuid(struct kvm_cpuid2 *kvm_cpuid)
> >                       entry->edx = signature[2];
> >                       break;
> >               case 1:
> > +                     entry->ebx &= ~(0xff << 24);
> > +                     entry->ebx |= cpu_id << 24;
> >                       /* Set X86_FEATURE_HYPERVISOR */
> >                       if (entry->index == 0)
> >                               entry->ecx |= (1 << 31);
> > @@ -80,7 +82,7 @@ void kvm_cpu__setup_cpuid(struct kvm_cpu *vcpu)
> >       if (ioctl(vcpu->kvm->sys_fd, KVM_GET_SUPPORTED_CPUID, kvm_cpuid) < 0)
> >               die_perror("KVM_GET_SUPPORTED_CPUID failed");
> >
> > -     filter_cpuid(kvm_cpuid);
> > +     filter_cpuid(kvm_cpuid, vcpu->cpu_id);
>
> Tested it and it works:
>
> Tested-by: Alexandru Elisei <alexandru.elisei@arm.com>
>
Thanks.