> 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.

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.

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.