Kernel requires x86-64 CPU, after modifying arch_shared_info struct

Kernel requires x86-64 CPU, after modifying arch_shared_info struct

[Jan Ruh]

[-- Attachment #1: Type: text/plain, Size: 1499 bytes --]

Hi Xen Dev Community,


I ran into an issue when implementing a prototype for a new paravirtualized clock for x86-64 hosts. I extended the arch_shared_info struct by 6 fields totaling at 36 bytes. I updated the xen-foreign/references.size to represent the new size of the arch_shared_info struct. The fields are correctly updated in Xen and I am also able to read the correct information stored from dom0. However, if I try to start a hvm VM with pvh extensions it does not boot telling me that "This kernel requires an x86-64 CPU, but only detected an i686 CPU.". I have rebuild my Linux domU kernel just as the dom0 kernel and everything should be compatible. To me this looks like Xen or libxc does some compatibility checks before booting up my HVM domU and decides to downgrade the detectable CPU due to some issue that I am not aware of. Do I miss something? Is my approach to extend the arch_shared_info wrong in the first place? I am really thankful for some advice or pointers what is happening here.


Best


Jan

CONFIDENTIALITY: The contents of this e-mail are confidential and intended only for the above addressee(s). If you are not the intended recipient, or the person responsible for delivering it to the intended recipient, copying or delivering it to anyone else or using it in any unauthorized manner is prohibited and may be unlawful. If you receive this e-mail by mistake, please notify the sender and the systems administrator at straymail@tttech.com immediately.

[-- Attachment #2: Type: text/html, Size: 1949 bytes --]

Re: Kernel requires x86-64 CPU, after modifying arch_shared_info struct

[Roger Pau Monné]

On Mon, Jun 29, 2020 at 07:43:43AM +0000, Jan Ruh wrote:
> Hi Xen Dev Community,
> 
> 
> I ran into an issue when implementing a prototype for a new
> paravirtualized clock for x86-64 hosts. I extended the
> arch_shared_info struct by 6 fields totaling at 36 bytes. I updated
> the xen-foreign/references.size to represent the new size of the
> arch_shared_info struct. The fields are correctly updated in Xen and
> I am also able to read the correct information stored from dom0.
> However, if I try to start a hvm VM with pvh extensions it does not
> boot telling me that "This kernel requires an x86-64 CPU, but only
> detected an i686 CPU.".

Did you try backing up your changes and seeing if the same happens?

Did you rebuild both the Xen hypervisor and the tools?

> I have rebuild my Linux domU kernel just as
> the dom0 kernel and everything should be compatible. To me this
> looks like Xen or libxc does some compatibility checks before
> booting up my HVM domU and decides to downgrade the detectable CPU
> due to some issue that I am not aware of. Do I miss something?

Pasting your xl config file would be helpful in order to help debug.

> Is my
> approach to extend the arch_shared_info wrong in the first place?

Doesn't look directly related to a change in arch_shared_info IMO, but
it's hard to tell without having more info.

Posting your changes might also help spot something wonky.

> CONFIDENTIALITY: The contents of this e-mail are confidential and
> intended only for the above addressee(s). If you are not the
> intended recipient, or the person responsible for delivering it to
> the intended recipient, copying or delivering it to anyone else or
> using it in any unauthorized manner is prohibited and may be
> unlawful. If you receive this e-mail by mistake, please notify the
> sender and the systems administrator at straymail@tttech.com
> immediately.

Nit: posting confidentiality banners to a public mailing lists
messages is kind of award, as the emails are publicly available for
anyone to read, even those that are not recipients of xen-devel, ie:

https://lists.xenproject.org/archives/html/xen-devel/2020-06/msg01868.html

Roger.

AW: Kernel requires x86-64 CPU, after modifying arch_shared_info struct

[Jan Ruh]

> Did you try backing up your changes and seeing if the same happens?

If backing up my changes everything works as expected.

> Did you rebuild both the Xen hypervisor and the tools?

Yes, I rebuild both Xen hypervisor and the tools.

> Pasting your xl config file would be helpful in order to help debug.

As requested my xl config:
    type="hvm"; extra="console=hvc0 earlyprintk=xen";
    kernel="/usr/lib/kernel/vmlinuz-domu";
    ramdisk="/usr/lib/kernel/initrd.img-domu";
    root="/dev/xvda2 ro";
    memory=1024;
    autoballoon="off";
    xen_platform_pci=1;
    pae=1; acpi=1; apic=1; vcpus=1;
    name="vm1";
    disk=["file:domu.img,hda,w"];
    vif=["bridge=xenbr0"];
    vfb=["type=vnc,keymap=de"];
    vnclisten="192.168.2.4:0";
    boot="c";'

> Posting your changes might also help spot something wonky.

diff --git a/xen/include/public/arch-x86/xen.h b/xen/include/public/arch-x86/xen.h
index 629cb2ba40..61c46504a5 100644
--- a/xen/include/public/arch-x86/xen.h
+++ b/xen/include/public/arch-x86/xen.h
@@ -265,6 +265,14 @@ struct arch_shared_info {
     /* There's no room for this field in the generic structure. */
     uint32_t wc_sec_hi;
 #endif
+
+    uint32_t st_version;
+    uint64_t time_sec;
+    uint64_t time_nsec;
+    uint64_t cycle_last;
+    uint32_t mult;
+    uint32_t shift;
+
 };
 typedef struct arch_shared_info arch_shared_info_t;

diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index c39fbe50a0..2782cb5127 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -1254,15 +1254,15 @@ void update_domain_synctime(struct domain *d)
 {
     uint32_t *st_version;

+    st_version = &shared_info(d, arch.st_version);
     *st_version = version_update_begin(*st_version);
     smp_wmb();

+    shared_info(d, arch.mult)        = global_time.mult;
+    shared_info(d, arch.shift)       = global_time.shift;
+    shared_info(d, arch.cycle_last)  = global_time.cycle_last;
+    shared_info(d, arch.time_sec)    = global_time.time_sec;
+    shared_info(d, arch.time_nsec)   = global_time.time_nsec;

     smp_wmb();
     *st_version = version_update_end(*st_version);

diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index 72e7d33708..4b9ad0261b 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -503,22 +503,22 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
     }
     case XEN_SYSCTL_adjust_gtime:
     {
+        struct adjust_gtime *adjust_gtime = (struct adjust_gtime*) &op->u.adjust_gtime;

+        ret = do_adj_gtime(adjust_gtime);
+
         copyback = 1;

         break;

diff --git a/tools/include/xen-foreign/reference.size b/tools/include/xen-foreign/reference.size
index bb2ada32fb..9afd11e5fa 100644
--- a/tools/include/xen-foreign/reference.size
+++ b/tools/include/xen-foreign/reference.size
@@ -9,6 +9,6 @@ vcpu_guest_context        |     344     344    2800    5168
 arch_vcpu_info            |       0       0      24      16
 vcpu_time_info            |      32      32      32      32
 vcpu_info                 |      48      48      64      64
-arch_shared_info          |       0       0      28      48
+arch_shared_info          |       0       0      64      88


global_time is a static struct in time.c, no existing Xen code is changed, just functions added that are being called from the sysctl adjust_gtime.

Further tests show that in /tools/libxc/xc_dom_binloader.c: xc_dom_parse_bin_kernel xc sets the dom->guest_type to "xen-3.0-x86_32" instead of "xen-3.0-x86_64". I also cannot see though how it can be connected to my change to arch_shared_info.

Sorry for the banner, I always forget that the mail client adds that thing, I hope it doesn't do it again.

Jan



CONFIDENTIALITY: The contents of this e-mail are confidential and intended only for the above addressee(s). If you are not the intended recipient, or the person responsible for delivering it to the intended recipient, copying or delivering it to anyone else or using it in any unauthorized manner is prohibited and may be unlawful. If you receive this e-mail by mistake, please notify the sender and the systems administrator at straymail@tttech.com immediately.

Re: Kernel requires x86-64 CPU, after modifying arch_shared_info struct

[Roger Pau Monné]

On Mon, Jun 29, 2020 at 09:56:43AM +0000, Jan Ruh wrote:
> > Did you try backing up your changes and seeing if the same happens?
> 
> If backing up my changes everything works as expected.
> 
> > Did you rebuild both the Xen hypervisor and the tools?
> 
> Yes, I rebuild both Xen hypervisor and the tools.
> 
> > Pasting your xl config file would be helpful in order to help debug.
> 
> As requested my xl config:

xl parser will just ignore the ';', you can remove them.

>     type="hvm"; extra="console=hvc0 earlyprintk=xen";
>     kernel="/usr/lib/kernel/vmlinuz-domu";
>     ramdisk="/usr/lib/kernel/initrd.img-domu";
>     root="/dev/xvda2 ro";
>     memory=1024;
>     autoballoon="off";

autoballoon is not a xl.cfg option.

>     xen_platform_pci=1;
>     pae=1; acpi=1; apic=1;

All those are already enabled by default, no need to specify them
here.

>     vcpus=1;
>     name="vm1";
>     disk=["file:domu.img,hda,w"];
>     vif=["bridge=xenbr0"];
>     vfb=["type=vnc,keymap=de"];
>     vnclisten="192.168.2.4:0";
>     boot="c";'
> 
> > Posting your changes might also help spot something wonky.
> 
> diff --git a/xen/include/public/arch-x86/xen.h b/xen/include/public/arch-x86/xen.h
> index 629cb2ba40..61c46504a5 100644
> --- a/xen/include/public/arch-x86/xen.h
> +++ b/xen/include/public/arch-x86/xen.h
> @@ -265,6 +265,14 @@ struct arch_shared_info {
>      /* There's no room for this field in the generic structure. */
>      uint32_t wc_sec_hi;
>  #endif
> +
> +    uint32_t st_version;
> +    uint64_t time_sec;
> +    uint64_t time_nsec;
> +    uint64_t cycle_last;
> +    uint32_t mult;
> +    uint32_t shift;
> +
>  };
>  typedef struct arch_shared_info arch_shared_info_t;
> 
> diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
> index c39fbe50a0..2782cb5127 100644
> --- a/xen/arch/x86/time.c
> +++ b/xen/arch/x86/time.c
> @@ -1254,15 +1254,15 @@ void update_domain_synctime(struct domain *d)

This doesn't seem to exist in current Xen code, so I guess there are
further changes applied here?

>  {
>      uint32_t *st_version;
> 
> +    st_version = &shared_info(d, arch.st_version);
>      *st_version = version_update_begin(*st_version);
>      smp_wmb();
> 
> +    shared_info(d, arch.mult)        = global_time.mult;
> +    shared_info(d, arch.shift)       = global_time.shift;
> +    shared_info(d, arch.cycle_last)  = global_time.cycle_last;
> +    shared_info(d, arch.time_sec)    = global_time.time_sec;
> +    shared_info(d, arch.time_nsec)   = global_time.time_nsec;
> 
>      smp_wmb();
>      *st_version = version_update_end(*st_version);
> 
> diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
> index 72e7d33708..4b9ad0261b 100644
> --- a/xen/common/sysctl.c
> +++ b/xen/common/sysctl.c
> @@ -503,22 +503,22 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
>      }
>      case XEN_SYSCTL_adjust_gtime:
>      {
> +        struct adjust_gtime *adjust_gtime = (struct adjust_gtime*) &op->u.adjust_gtime;
> 
> +        ret = do_adj_gtime(adjust_gtime);

Same with do_adj_gtime, I cannot find it in the current code, hence
I'm afraid it's impossible to tell what it's actually doing.

> +
>          copyback = 1;
> 
>          break;
> 
> diff --git a/tools/include/xen-foreign/reference.size b/tools/include/xen-foreign/reference.size
> index bb2ada32fb..9afd11e5fa 100644
> --- a/tools/include/xen-foreign/reference.size
> +++ b/tools/include/xen-foreign/reference.size
> @@ -9,6 +9,6 @@ vcpu_guest_context        |     344     344    2800    5168
>  arch_vcpu_info            |       0       0      24      16
>  vcpu_time_info            |      32      32      32      32
>  vcpu_info                 |      48      48      64      64
> -arch_shared_info          |       0       0      28      48
> +arch_shared_info          |       0       0      64      88

Aren't you missing a line below that contains the shared_info size,
and that also need to be updated (since arch_shared_info is contained
in shared_info)?

> 
> 
> global_time is a static struct in time.c, no existing Xen code is changed, just functions added that are being called from the sysctl adjust_gtime.
> 
> Further tests show that in /tools/libxc/xc_dom_binloader.c: xc_dom_parse_bin_kernel xc sets the dom->guest_type to "xen-3.0-x86_32" instead of "xen-3.0-x86_64". I also cannot see though how it can be connected to my change to arch_shared_info.

Hm, I think guest type should be hvm-3.0-x86_32, as xen-* are PV guest
types, and you are trying to boot a HVM guest.

Anyway I'm not familiar with HVM direct kernel boot, so this might
have no effect here.

Are you sure the type is set to "xen-3.0-x86_64" prior to your
changes?

Maybe it would be worth to also paste the output of 'xl -vvv create
...'.

> Sorry for the banner, I always forget that the mail client adds that thing, I hope it doesn't do it again.

I'm afraid it's still appended to this email, see below.

Roger.

> CONFIDENTIALITY: The contents of this e-mail are confidential and
> intended only for the above addressee(s). If you are not the
> intended recipient, or the person responsible for delivering it to
> the intended recipient, copying or delivering it to anyone else or
> using it in any unauthorized manner is prohibited and may be
> unlawful. If you receive this e-mail by mistake, please notify the
> sender and the systems administrator at straymail@tttech.com
> immediately.

AW: Kernel requires x86-64 CPU, after modifying arch_shared_info struct

[Jan Ruh]

Hi Roger,

thanks for your time, your remark that you cannot see how it has to do with the shared_info got me to step-by-step revert all changes I have done. Turns out a few weeks ago when implementing a sysctl I must have accidentally deleted the architecture specific syctl in the default case of the common sysctl. This leads to Xen quietly defaulting to a x86_32 CPU.

Jan.
CONFIDENTIALITY: The contents of this e-mail are confidential and intended only for the above addressee(s). If you are not the intended recipient, or the person responsible for delivering it to the intended recipient, copying or delivering it to anyone else or using it in any unauthorized manner is prohibited and may be unlawful. If you receive this e-mail by mistake, please notify the sender and the systems administrator at straymail@tttech.com immediately.