Re: [Xen-devel] [PATCH v2 1/2] nvmx: implement support for MSR bitmaps

[Tian, Kevin]

> From: Roger Pau Monne 
> Sent: Wednesday, January 29, 2020 10:45 PM
> 
> Current implementation of nested VMX has a half baked handling of MSR
> bitmaps for the L1 VMM: it maps the L1 VMM provided MSR bitmap, but
> doesn't actually load it into the nested vmcs, and thus the nested
> guest vmcs ends up using the same MSR bitmap as the L1 VMM.
> 
> This is wrong as there's no assurance that the set of features enabled
> for the L1 vmcs are the same that L1 itself is going to use in the
> nested vmcs, and thus can lead to misconfigurations.
> 
> For example L1 vmcs can use x2APIC virtualization and virtual
> interrupt delivery, and thus some x2APIC MSRs won't be trapped so that
> they can be handled directly by the hardware using virtualization
> extensions. On the other hand, the nested vmcs created by L1 VMM might
> not use any of such features, so using a MSR bitmap that doesn't trap
> accesses to the x2APIC MSRs will be leaking them to the underlying
> hardware.
> 
> Fix this by crafting a merged MSR bitmap between the one used by L1
> and the nested guest.
> 
> Signed-off-by: Roger Pau Monné 
> ---
> This seems better than what's done currently, but TBH there's a lot of
> work to be done in nvmx in order to make it functional and secure that
> I'm not sure whether building on top of the current implementation is
> something sane to do, or it would be better to start from scratch and
> re-implement nvmx to just support the minimum required set of VTx
> features in a sane and safe way.

without knowing what "a lot of work" actually means, it's difficult to 
judge which way is better. But from the listed changes in this series,
I think they are reasonable.

> ---
> Changes since v1:
>  - Split the x2APIC MSR fix into a separate patch.
>  - Move setting MSR_BITMAP vmcs field into load_vvmcs_host_state for
>virtual vmexit.
>  - Allocate memory with MEMF_no_owner.
>  - Use tabs to align comment of the nestedvmx struct field.
> ---
>  xen/arch/x86/hvm/vmx/vvmx.c| 63 --
>  xen/include/asm-x86/hvm/vmx/vvmx.h |  3 +-
>  2 files changed, 62 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
> index 47eee1e5b9..c35b4bab84 100644
> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> @@ -128,6 +128,16 @@ int nvmx_vcpu_initialise(struct vcpu *v)
>  unmap_domain_page(vw);
>  }
> 
> +if ( cpu_has_vmx_msr_bitmap )
> +{
> +nvmx->msr_merged = alloc_domheap_page(d, MEMF_no_owner);
> +if ( !nvmx->msr_merged )
> +{
> +gdprintk(XENLOG_ERR, "nest: allocation for MSR bitmap failed\n");
> +return -ENOMEM;
> +}
> +}
> +
>  nvmx->ept.enabled = 0;
>  nvmx->guest_vpid = 0;
>  nvmx->vmxon_region_pa = INVALID_PADDR;
> @@ -182,6 +192,11 @@ void nvmx_vcpu_destroy(struct vcpu *v)
>  free_domheap_page(v->arch.hvm.vmx.vmwrite_bitmap);
>  v->arch.hvm.vmx.vmwrite_bitmap = NULL;
>  }
> +if ( nvmx->msr_merged )
> +{
> +free_domheap_page(nvmx->msr_merged);
> +nvmx->msr_merged = NULL;
> +}
>  }
> 
>  void nvmx_domain_relinquish_resources(struct domain *d)
> @@ -548,6 +563,37 @@ unsigned long *_shadow_io_bitmap(struct vcpu *v)
>  return nestedhvm_vcpu_iomap_get(port80, portED);
>  }
> 
> +static void update_msrbitmap(struct vcpu *v)
> +{
> +struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
> +struct vmx_msr_bitmap *msr_bitmap;
> +unsigned int msr;
> +
> +ASSERT(__n2_exec_control(v) & CPU_BASED_ACTIVATE_MSR_BITMAP);

what about passing shadow_cntrl and also moving the outer 
condition check into this function? It is not good to assume
that __n2_exec_control always has the same setting as the
local variable shadow_cntrl.

> +
> +if ( !nvmx->msrbitmap )
> +return;
> +
> +msr_bitmap = __map_domain_page(nvmx->msr_merged);
> +
> +bitmap_or(msr_bitmap->read_low, nvmx->msrbitmap->read_low,
> +  v->arch.hvm.vmx.msr_bitmap->read_low,
> +  sizeof(msr_bitmap->read_low) * 8);
> +bitmap_or(msr_bitmap->read_high, nvmx->msrbitmap->read_high,
> +  v->arch.hvm.vmx.msr_bitmap->read_high,
> +  sizeof(msr_bitmap->read_high) * 8);
> +bitmap_or(msr_bitmap->write_low, nvmx->msrbitmap->write_low,
> +  v->arch.hvm.vmx.msr_bitmap->write_low,
> +  sizeof(msr_bitmap->write_low) * 8);
> +bitmap_or(msr_bitmap->write_high, nvmx->msrbitmap->write_high,
> +  v->arch.hvm.vmx.msr_bitmap->write_high,
> +  sizeof(msr_bitmap->write_high) * 8);
> +
> +unmap_domain_page(msr_bitmap);
> +
> +__vmwrite(MSR_BITMAP, page_to_maddr(nvmx->msr_merged));
> +}
> +
>  void nvmx_update_exec_control(struct vcpu *v, u32 host_cntrl)
>  {
>  u32 pio_cntrl = (CPU_BASED_ACTIVATE_IO_BITMAP
> @@ -558,10 +604,15 @@ void nvmx_update_exec_control(struct vcpu *v,
> u32 host_cntrl)
>  shadow_cntrl = __n2_exec

Re: [Xen-devel] [PATCH v2 2/2] nvmx: always trap accesses to x2APIC MSRs

[Tian, Kevin]

> From: Roger Pau Monne 
> Sent: Wednesday, January 29, 2020 10:45 PM
> 
> Nested VMX doesn't expose support for
> SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE,
> SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY or
> SECONDARY_EXEC_APIC_REGISTER_VIRT, and hence the x2APIC MSRs should
> always be trapped in the nested guest MSR bitmap, or else a nested
> guest could access the hardware x2APIC MSRs given certain conditions.
> 
> Accessing the hardware MSRs could be achieved by forcing the L0 Xen to
> use SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE and
> SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY or
> SECONDARY_EXEC_APIC_REGISTER_VIRT (if supported), and then creating a
> L2 guest with a MSR bitmap that doesn't trap accesses to the x2APIC
> MSR range. Then OR'ing both L0 and L1 MSR bitmaps would result in a
> bitmap that doesn't trap certain x2APIC MSRs and a VMCS that doesn't
> have SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE and
> SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY or
> SECONDARY_EXEC_APIC_REGISTER_VIRT set either.
> 
> Fix this by making sure x2APIC MSRs are always trapped in the nested
> MSR bitmap.
> 
> Signed-off-by: Roger Pau Monné 

Reviewed-by: Kevin Tian 
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v7 1/3] x86 / vmx: move teardown from domain_destroy()...

[Tian, Kevin]

> From: Paul Durrant 
> Sent: Thursday, January 30, 2020 1:10 AM
> 
> ... to domain_relinquish_resources().
> 
> The teardown code frees the APICv page. This does not need to be done late
> so do it in domain_relinquish_resources() rather than domain_destroy().
> 
> Signed-off-by: Paul Durrant 

Reviewed-by: Kevin Tian 
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v8 4/4] x86 / vmx: use a MEMF_no_refcount domheap page for APIC_DEFAULT_PHYS_BASE

[Tian, Kevin]

> From: Paul Durrant 
> Sent: Thursday, January 30, 2020 10:58 PM
> 
> vmx_alloc_vlapic_mapping() currently contains some very odd looking code
> that allocates a MEMF_no_owner domheap page and then shares with the
> guest
> as if it were a xenheap page. This then requires vmx_free_vlapic_mapping()
> to call a special function in the mm code: free_shared_domheap_page().
> 
> By using a MEMF_no_refcount domheap page instead, the odd looking code
> in
> vmx_alloc_vlapic_mapping() can simply use get_page_and_type() to set up a
> writable mapping before insertion in the P2M and
> vmx_free_vlapic_mapping()
> can simply release the page using put_page_alloc_ref() followed by
> put_page_and_type(). This then allows free_shared_domheap_page() to be
> purged.
> 
> Signed-off-by: Paul Durrant 
> Reviewed-by: Jan Beulich 

Reviewed-by: Kevin Tian 
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [ovmf test] 146686: regressions - FAIL

[osstest service owner]

flight 146686 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/146686/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 145767
 test-amd64-amd64-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 
145767

version targeted for testing:
 ovmf eafd990f2606431d45cf0bbdbfee6d5959628de7
baseline version:
 ovmf 70911f1f4aee0366b6122f2b90d367ec0f066beb

Last test of basis   145767  2020-01-08 00:39:09 Z   26 days
Failing since145774  2020-01-08 02:50:20 Z   26 days   97 attempts
Testing same since   146634  2020-02-01 01:54:23 Z2 days8 attempts


People who touched revisions under test:
  Aaron Li 
  Albecki, Mateusz 
  Anthony PERARD 
  Ard Biesheuvel 
  Ashish Singhal 
  Bob Feng 
  Brian R Haug 
  Eric Dong 
  Fan, ZhijuX 
  Hao A Wu 
  Jason Voelz 
  Jian J Wang 
  Krzysztof Koch 
  Laszlo Ersek 
  Leif Lindholm 
  Li, Aaron 
  Liming Gao 
  Mateusz Albecki 
  Michael D Kinney 
  Michael Kubacki 
  Pavana.K 
  Philippe Mathieu-Daud? 
  Philippe Mathieu-Daude 
  Siyuan Fu 
  Siyuan, Fu 
  Sudipto Paul 
  Vitaly Cheptsov 
  Vitaly Cheptsov via Groups.Io 
  Wei6 Xu 
  Xu, Wei6 
  Zhiguang Liu 
  Zhiju.Fan 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 fail
 test-amd64-i386-xl-qemuu-ovmf-amd64  fail



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Not pushing.

(No revision log; it would be 1780 lines long.)

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] Xen-unstable: pci-passthrough regression bisected to: x86/smp: use APIC ALLBUT destination shorthand when possible

[Sander Eikelenboom]

Hi Roger,

Last week I encountered an issue with the PCI-passthrough of a USB controller. 
In the guest I get:
[ 1143.313756] xhci_hcd :00:05.0: xHCI host not responding to stop 
endpoint command.
[ 1143.334825] xhci_hcd :00:05.0: xHCI host controller not responding, 
assume dead
[ 1143.347364] xhci_hcd :00:05.0: HC died; cleaning up
[ 1143.356407] usb 1-2: USB disconnect, device number 2

Bisection turned up as the culprit: 
   commit 5500d265a2a8fa63d60c08beb549de8ec82ff7a5
   x86/smp: use APIC ALLBUT destination shorthand when possible

I verified by reverting that commit and now it works fine again.

Box is AMD, guest is a HVM.

--
Sander

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 4/9] xen: add basic hypervisor filesystem support

[Jürgen Groß]

On 31.01.20 16:50, Wei Liu wrote:

On Tue, Jan 21, 2020 at 09:43:25AM +0100, Juergen Gross wrote:
[...]

diff --git a/xen/common/hypfs.c b/xen/common/hypfs.c
new file mode 100644
index 00..6762d20dfd
--- /dev/null
+++ b/xen/common/hypfs.c
@@ -0,0 +1,365 @@
+/**
+ *
+ * hypfs.c
+ *
+ * Simple sysfs-like file system for the hypervisor.
+ */
+
+#include 


This should come after hypfs.h.

If it has come first it probably means one of the headers below hasn't
included it properly?


I'll move it, as the build will still be fine.


Juergen

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 5/9] libs: add libxenhypfs

[Jürgen Groß]

On 31.01.20 16:57, Wei Liu wrote:

On Tue, Jan 21, 2020 at 09:43:26AM +0100, Juergen Gross wrote:

Add the new library libxenhypfs for access to the hypervisor filesystem.

Signed-off-by: Juergen Gross 


I've only skimmed read it. The code looks sensible.

One minor comment.


diff --git a/tools/libs/hypfs/xenhypfs.pc.in b/tools/libs/hypfs/xenhypfs.pc.in
new file mode 100644
index 00..9cb968f0db
--- /dev/null
+++ b/tools/libs/hypfs/xenhypfs.pc.in
@@ -0,0 +1,10 @@
+prefix=@@prefix@@
+includedir=@@incdir@@
+libdir=@@libdir@@
+
+Name: Xenhypfs
+Description: The Xenhypfs library for Xen hypervisor
+Version: @@version@@
+Cflags: -I${includedir} @@cflagslocal@@
+Libs: @@libsflag@@${libdir} -lxenhypfs
+Requires.private: xentoolcore,xentoollog,xencall


Need to list libz here?


Probably, yes.


Juergen

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [linux-5.4 test] 146682: regressions - FAIL

[osstest service owner]

flight 146682 linux-5.4 real [real]
http://logs.test-lab.xenproject.org/osstest/logs/146682/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 
146121
 test-amd64-i386-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 146121
 test-amd64-amd64-qemuu-nested-intel 17 debian-hvm-install/l1/l2 fail REGR. vs. 
146121
 build-i386-xsm6 xen-build  fail in 146675 REGR. vs. 146121

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-qemuu-nested-amd  7 xen-bootfail in 146675 pass in 146682
 test-amd64-amd64-xl-rtds 15 guest-saverestore  fail pass in 146667
 test-amd64-i386-qemut-rhel6hvm-amd 12 guest-start/redhat.repeat fail pass in 
146675

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-rtds 18 guest-localmigrate/x10 fail in 146667 REGR. vs. 
146121

Tests which did not succeed, but are not blocking:
 test-amd64-i386-libvirt-xsm   1 build-check(1)   blocked in 146675 n/a
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 1 build-check(1) blocked 
in 146675 n/a
 test-amd64-i386-xl-qemut-debianhvm-i386-xsm 1 build-check(1) blocked in 146675 
n/a
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked in 
146675 n/a
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 1 build-check(1) blocked in 146675 
n/a
 test-amd64-i386-xl-xsm1 build-check(1)   blocked in 146675 n/a
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop  fail never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail

Re: [Xen-devel] [PATCH v4 1/7] SVM: drop asm/hvm/emulate.h inclusion from vmcb.h

[Alexandru Stefan ISAILA]

On 31.01.2020 18:42, Jan Beulich wrote:
> It's not needed there and introduces a needless, almost global
> dependency. Include the file (or in some cases just xen/err.h) where
> actually needed, or - in one case - simply forward-declare a struct. In
> microcode*.c take the opportunity and also re-order a few other
> #include-s.
> 
> Signed-off-by: Jan Beulich 

Reviewed-by: Alexandru Isaila 
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] docs/designs: Add a design document for transparent live migration

[David Woodhouse]

On Mon, 2020-01-27 at 16:46 +, Ian Jackson wrote:
> I don't like the term `transparent'.  It is often abused in other
> contexts.  It can be clear to whom things are transparent.  In a very
> real sense existing migration is `transparent' to a domain's network
> peers, for example.  How about `oblivious' ?

The term we generally use is 'guest transparent live migration', in
which the additional word addresses that potential ambiguity. We thus
have GT migration in addition to SR (suspend/resume) migration.

Perhaps it's just familiarity, but I very much prefer that to
'oblivious' (guests aren't necessary oblivious to it; they just aren't
required to *do* anything). and to 'non-cooperative' which for me is
too easy to conflate with 'uncooperative' and might cause an inference
that it's for guests which have done something *wrong*.



smime.p7s
Description: S/MIME cryptographic signature
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [qemu-mainline test] 146692: regressions - FAIL

[osstest service owner]

flight 146692 qemu-mainline real [real]
http://logs.test-lab.xenproject.org/osstest/logs/146692/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-arm64   6 xen-buildfail REGR. vs. 144861
 build-arm64-xsm   6 xen-buildfail REGR. vs. 144861
 build-armhf   6 xen-buildfail REGR. vs. 144861
 build-amd64   6 xen-buildfail REGR. vs. 144861
 build-amd64-xsm   6 xen-buildfail REGR. vs. 144861
 build-i386-xsm6 xen-buildfail REGR. vs. 144861
 build-i3866 xen-buildfail REGR. vs. 144861

Tests which did not succeed, but are not blocking:
 test-amd64-i386-xl-qemuu-win7-amd64  1 build-check(1)  blocked n/a
 test-amd64-amd64-xl-pvshim1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-credit1   1 build-check(1)   blocked  n/a
 test-amd64-amd64-pygrub   1 build-check(1)   blocked  n/a
 test-amd64-i386-xl1 build-check(1)   blocked  n/a
 build-armhf-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-i386-qemuu-rhel6hvm-intel  1 build-check(1) blocked n/a
 test-arm64-arm64-xl-credit1   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-pvhv2-intel  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-credit1   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qcow2 1 build-check(1)   blocked  n/a
 test-amd64-amd64-i386-pvgrub  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-xsm   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl   1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-amd64  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-credit2   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ws16-amd64  1 build-check(1) blocked n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked 
n/a
 test-amd64-amd64-xl-rtds  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-cubietruck  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-shadow1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-amd64-i386-xl-xsm1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow  1 build-check(1) blocked n/a
 test-amd64-amd64-xl-qemuu-win7-amd64  1 build-check(1) blocked n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-ws16-amd64  1 build-check(1)  blocked n/a
 test-arm64-arm64-xl   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  1 build-check(1)blocked n/a
 test-amd64-i386-xl-pvshim 1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-raw1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked 
n/a
 test-amd64-i386-freebsd10-i386  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-seattle   1 build-check(1)   blocked  n/a
 test-amd64-i386-pair  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-arndale   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked  n/a
 build-arm64-libvirt   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-credit2   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-amd64-amd64-qemuu-nested-amd  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-qemuu-rhel6hvm-amd  1 build-check(1)   blocked n/a
 test-amd64-amd64-pair 1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-i3

Re: [Xen-devel] [PATCH v3 3/9] docs: add feature document for Xen hypervisor sysfs-like support

[Julien Grall]

On 21/01/2020 14:17, Jürgen Groß wrote:

On 21.01.20 14:14, Julien Grall wrote:
However, I am not sure what are actually the tags? Do you have a 
concrete example how they can be used?


I'll add this one:

/cpu-bugs/active-pv/xpti (0|1) [w,X86,PV]




+
+* w -- Path is writable by the user. This capability is usually
+  limited to the control domain (e.g. dom0).
+* ARM | ARM32 | X86: the path is available for the respective 
architecture

+  only.


How about Arm64? Also, if it is support by both arm64 and arm32, 
should we use ARM or ARM32,ARM64?


ARM64 should be added and I'd suggest to use "ARM" instead of
"ARM32,ARM64".


I am happy with that.

Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH] docs/xl: fix typo in xl.cfg

[Roger Pau Monne]

The name of the option is nographic.

Signed-off-by: Roger Pau Monné 
---
 docs/man/xl.cfg.5.pod.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
index 245d3f9472..0cad561375 100644
--- a/docs/man/xl.cfg.5.pod.in
+++ b/docs/man/xl.cfg.5.pod.in
@@ -2504,7 +2504,7 @@ Available options are: B.
 Redirect virtual serial ports to Bs. Please see the
 B<-serial> option in the B manpage for details of the valid
 B options. Default is B when in graphical mode and
-B if B is used.
+B if B is used.
 
 The form serial=DEVICE is also accepted for backwards compatibility.
 
-- 
2.25.0


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v6 05/11] x86/hyperv: setup hypercall page

[Durrant, Paul]

> -Original Message-
> From: Wei Liu 
> Sent: 31 January 2020 17:57
> To: Xen Development List 
> Cc: Durrant, Paul ; Michael Kelley
> ; Wei Liu ; Wei Liu
> ; Jan Beulich ; Andrew Cooper
> ; Roger Pau Monné 
> Subject: Re: [PATCH v6 05/11] x86/hyperv: setup hypercall page
> 
> (Note to self)
> 
> On Fri, Jan 31, 2020 at 05:49:24PM +, Wei Liu wrote:
> [...]
> > +static uint64_t generate_guest_id(void)
> > +{
> > +union hv_guest_os_id id;
> > +
> 
>id.raw = 0;

Or just use a C99 initializer to set things up. A bit neater IMO.

  Paul

> 
> > +id.vendor = HV_XEN_VENDOR_ID;
> > +id.major = xen_major_version();
> > +id.minor = xen_minor_version();
> > +
> > +return id.raw;
> > +}

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v9 1/4] x86 / vmx: move teardown from domain_destroy()...

[Paul Durrant]

... to domain_relinquish_resources().

The teardown code frees the APICv page. This does not need to be done late
so do it in domain_relinquish_resources() rather than domain_destroy().

Signed-off-by: Paul Durrant 
Reviewed-by: Jan Beulich 
Reviewed-by: Kevin Tian 
---
Cc: Jun Nakajima 
Cc: Andrew Cooper 
Cc: Wei Liu 
Cc: "Roger Pau Monné" 
Cc: George Dunlap 

v4:
  - New in v4 (disaggregated from v3 patch #3)
---
 xen/arch/x86/hvm/vmx/vmx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index b262d38a7c..606f3dc2eb 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -419,7 +419,7 @@ static int vmx_domain_initialise(struct domain *d)
 return 0;
 }
 
-static void vmx_domain_destroy(struct domain *d)
+static void vmx_domain_relinquish_resources(struct domain *d)
 {
 if ( !has_vlapic(d) )
 return;
@@ -2240,7 +2240,7 @@ static struct hvm_function_table __initdata 
vmx_function_table = {
 .cpu_up_prepare   = vmx_cpu_up_prepare,
 .cpu_dead = vmx_cpu_dead,
 .domain_initialise= vmx_domain_initialise,
-.domain_destroy   = vmx_domain_destroy,
+.domain_relinquish_resources = vmx_domain_relinquish_resources,
 .vcpu_initialise  = vmx_vcpu_initialise,
 .vcpu_destroy = vmx_vcpu_destroy,
 .save_cpu_ctxt= vmx_save_vmcs_ctxt,
-- 
2.20.1


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v9 3/4] mm: make pages allocated with MEMF_no_refcount safe to assign

[Paul Durrant]

Currently it is unsafe to assign a domheap page allocated with
MEMF_no_refcount to a domain because the domain't 'tot_pages' will not
be incremented, but will be decrement when the page is freed (since
free_domheap_pages() has no way of telling that the increment was skipped).

This patch allocates a new 'count_info' bit for a PGC_extra flag
which is then used to mark pages when alloc_domheap_pages() is called
with MEMF_no_refcount. assign_pages() because it still needs to call
domain_adjust_tot_pages() to make sure the domain is appropriately
referenced. Hence it is modified to do that for PGC_extra pages even if it
is passed MEMF_no_refount.

The number of PGC_extra pages assigned to a domain is tracked in a new
'extra_pages' counter, which is then subtracted from 'total_pages' in
the domain_tot_pages() helper. Thus 'normal' page assignments will still
be appropriately checked against 'max_pages'.

Signed-off-by: Paul Durrant 
Reviewed-by: Jan Beulich 
---
Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Julien Grall 
Cc: Konrad Rzeszutek Wilk 
Cc: Stefano Stabellini 
Cc: Wei Liu 
Cc: Volodymyr Babchuk 
Cc: "Roger Pau Monné" 

v8:
 - Drop the idea of post-allocation assignment adding an error path to
   steal_page() if it encounters a PGC_extra page
 - Tighten up the ASSERTs in assign_pages()

v7:
 - s/PGC_no_refcount/PGC_extra/g
 - Re-work allocation to account for 'extra' pages, also making it
   safe to assign PGC_extra pages post-allocation

v6:
 - Add an extra ASSERT into assign_pages() that PGC_no_refcount is not
   set if MEMF_no_refcount is clear
 - ASSERT that count_info is 0 in alloc_domheap_pages() and set to
   PGC_no_refcount rather than ORing

v5:
 - Make sure PGC_no_refcount is set before assign_pages() is called
 - Don't bother to clear PGC_no_refcount in free_domheap_pages() and
   drop ASSERT in free_heap_pages()
 - Don't latch count_info in free_heap_pages()

v4:
 - New in v4
---
 xen/arch/x86/mm.c|  3 +-
 xen/common/page_alloc.c  | 63 +++-
 xen/include/asm-arm/mm.h |  5 +++-
 xen/include/asm-x86/mm.h |  7 +++--
 xen/include/xen/sched.h  |  5 +++-
 5 files changed, 64 insertions(+), 19 deletions(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index e1b041e2df..fd134edcde 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4217,7 +4217,8 @@ int steal_page(
 if ( !(owner = page_get_owner_and_reference(page)) )
 goto fail;
 
-if ( owner != d || is_xen_heap_page(page) )
+if ( owner != d || is_xen_heap_page(page) ||
+ (page->count_info & PGC_extra) )
 goto fail_put;
 
 /*
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index bbd3163909..1ac9d9c719 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -2267,7 +2267,29 @@ int assign_pages(
 goto out;
 }
 
-if ( !(memflags & MEMF_no_refcount) )
+#ifndef NDEBUG
+{
+unsigned int extra_pages = 0;
+
+for ( i = 0; i < (1ul << order); i++ )
+{
+ASSERT(!(pg[i].count_info & ~PGC_extra));
+if ( pg[i].count_info & PGC_extra )
+extra_pages++;
+}
+
+ASSERT(!extra_pages ||
+   ((memflags & MEMF_no_refcount) &&
+extra_pages == 1u << order));
+}
+#endif
+
+if ( pg[0].count_info & PGC_extra )
+{
+d->extra_pages += 1u << order;
+memflags &= ~MEMF_no_refcount;
+}
+else if ( !(memflags & MEMF_no_refcount) )
 {
 unsigned int tot_pages = domain_tot_pages(d) + (1 << order);
 
@@ -2278,18 +2300,19 @@ int assign_pages(
 rc = -E2BIG;
 goto out;
 }
+}
 
-if ( unlikely(domain_adjust_tot_pages(d, 1 << order) == (1 << order)) )
+if ( !(memflags & MEMF_no_refcount) &&
+ unlikely(domain_adjust_tot_pages(d, 1 << order) == (1 << order)) )
 get_knownalive_domain(d);
-}
 
 for ( i = 0; i < (1 << order); i++ )
 {
 ASSERT(page_get_owner(&pg[i]) == NULL);
-ASSERT(!pg[i].count_info);
 page_set_owner(&pg[i], d);
 smp_wmb(); /* Domain pointer must be visible before updating refcnt. */
-pg[i].count_info = PGC_allocated | 1;
+pg[i].count_info =
+(pg[i].count_info & PGC_extra) | PGC_allocated | 1;
 page_list_add_tail(&pg[i], &d->page_list);
 }
 
@@ -2315,11 +2338,6 @@ struct page_info *alloc_domheap_pages(
 
 if ( memflags & MEMF_no_owner )
 memflags |= MEMF_no_refcount;
-else if ( (memflags & MEMF_no_refcount) && d )
-{
-ASSERT(!(memflags & MEMF_no_refcount));
-return NULL;
-}
 
 if ( !dma_bitsize )
 memflags &= ~MEMF_no_dma;
@@ -2332,11 +2350,23 @@ struct page_info *alloc_domheap_pages(
   memflags, d)) == NULL)) )
  return NULL;
 
-if ( d && !(memflags & MEMF_no_owner) &&
- assign_pages(d, pg, order, memflags) )
+if ( d

[Xen-devel] [PATCH v9 2/4] add a domain_tot_pages() helper function

[Paul Durrant]

This patch adds a new domain_tot_pages() inline helper function into
sched.h, which will be needed by a subsequent patch.

No functional change.

NOTE: While modifying the comment for 'tot_pages' in sched.h this patch
  makes some cosmetic fixes to surrounding comments.

Suggested-by: Jan Beulich 
Signed-off-by: Paul Durrant 
---
Cc: Andrew Cooper 
Cc: Wei Liu 
Cc: "Roger Pau Monné" 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Julien Grall 
Cc: Konrad Rzeszutek Wilk 
Cc: Stefano Stabellini 
Cc: George Dunlap 
Cc: Tim Deegan 

v9:
 - Fix missing changes in PV shim
 - Dropped some comment changes

v8:
 - New in v8
---
 xen/arch/arm/arm64/domctl.c |  2 +-
 xen/arch/x86/domain.c   |  2 +-
 xen/arch/x86/mm.c   |  2 +-
 xen/arch/x86/mm/p2m-pod.c   | 10 +-
 xen/arch/x86/mm/shadow/common.c |  2 +-
 xen/arch/x86/msi.c  |  2 +-
 xen/arch/x86/numa.c |  2 +-
 xen/arch/x86/pv/dom0_build.c| 25 +
 xen/arch/x86/pv/domain.c|  2 +-
 xen/arch/x86/pv/shim.c  |  4 ++--
 xen/common/domctl.c |  2 +-
 xen/common/grant_table.c|  4 ++--
 xen/common/keyhandler.c |  2 +-
 xen/common/memory.c |  2 +-
 xen/common/page_alloc.c | 15 ---
 xen/include/public/memory.h |  4 ++--
 xen/include/xen/sched.h | 24 ++--
 17 files changed, 60 insertions(+), 46 deletions(-)

diff --git a/xen/arch/arm/arm64/domctl.c b/xen/arch/arm/arm64/domctl.c
index ab8781fb91..0de89b42c4 100644
--- a/xen/arch/arm/arm64/domctl.c
+++ b/xen/arch/arm/arm64/domctl.c
@@ -18,7 +18,7 @@ static long switch_mode(struct domain *d, enum domain_type 
type)
 
 if ( d == NULL )
 return -EINVAL;
-if ( d->tot_pages != 0 )
+if ( domain_tot_pages(d) != 0 )
 return -EBUSY;
 if ( d->arch.type == type )
 return 0;
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 28fefa1f81..643c23ffb0 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -218,7 +218,7 @@ void dump_pageframe_info(struct domain *d)
 
 printk("Memory pages belonging to domain %u:\n", d->domain_id);
 
-if ( d->tot_pages >= 10 && d->is_dying < DOMDYING_dead )
+if ( domain_tot_pages(d) >= 10 && d->is_dying < DOMDYING_dead )
 {
 printk("DomPage list too long to display\n");
 }
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index f50c065af3..e1b041e2df 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4870,7 +4870,7 @@ long arch_memory_op(unsigned long cmd, 
XEN_GUEST_HANDLE_PARAM(void) arg)
 else if ( rc >= 0 )
 {
 p2m = p2m_get_hostp2m(d);
-target.tot_pages   = d->tot_pages;
+target.tot_pages   = domain_tot_pages(d);
 target.pod_cache_pages = p2m->pod.count;
 target.pod_entries = p2m->pod.entry_count;
 
diff --git a/xen/arch/x86/mm/p2m-pod.c b/xen/arch/x86/mm/p2m-pod.c
index 096e2773fb..f2c9409568 100644
--- a/xen/arch/x86/mm/p2m-pod.c
+++ b/xen/arch/x86/mm/p2m-pod.c
@@ -302,7 +302,7 @@ out:
  * The following equations should hold:
  *  0 <= P <= T <= B <= M
  *  d->arch.p2m->pod.entry_count == B - P
- *  d->tot_pages == P + d->arch.p2m->pod.count
+ *  domain_tot_pages(d) == P + d->arch.p2m->pod.count
  *
  * Now we have the following potential cases to cover:
  * B tot_pages - p2m->pod.count;
+populated = domain_tot_pages(d) - p2m->pod.count;
 if ( populated > 0 && p2m->pod.entry_count == 0 )
 goto out;
 
@@ -348,7 +348,7 @@ p2m_pod_set_mem_target(struct domain *d, unsigned long 
target)
  * T' < B: Don't reduce the cache size; let the balloon driver
  * take care of it.
  */
-if ( target < d->tot_pages )
+if ( target < domain_tot_pages(d) )
 goto out;
 
 pod_target = target - populated;
@@ -1231,8 +1231,8 @@ out_of_memory:
 pod_unlock(p2m);
 
 printk("%s: Dom%d out of PoD memory! (tot=%"PRIu32" ents=%ld dom%d)\n",
-   __func__, d->domain_id, d->tot_pages, p2m->pod.entry_count,
-   current->domain->domain_id);
+   __func__, d->domain_id, domain_tot_pages(d),
+   p2m->pod.entry_count, current->domain->domain_id);
 domain_crash(d);
 return false;
 out_fail:
diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c
index 6212ec2c4a..cba3ab1eba 100644
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -1256,7 +1256,7 @@ static unsigned int sh_min_allocation(const struct domain 
*d)
  * up of slot zero and an LAPIC page), plus one for HVM's 1-to-1 pagetable.
  */
 return shadow_min_acceptable_pages(d) +
-   max(max(d->tot_pages / 256,
+   max(max(domain_tot_pages(d) / 256,
is_hvm_domain(d) ? CONFIG_PAGING_LEVELS + 2 : 0U) +
is_hvm_domain(d),
d->arch.paging.shadow.p2m_pages);
diff --git a/xen/arch/x86/msi.c b/xen/arc

[Xen-devel] [PATCH v9 0/4] purge free_shared_domheap_page()

[Paul Durrant]

Paul Durrant (4):
  x86 / vmx: move teardown from domain_destroy()...
  add a domain_tot_pages() helper function
  mm: make pages allocated with MEMF_no_refcount safe to assign
  x86 / vmx: use a MEMF_no_refcount domheap page for
APIC_DEFAULT_PHYS_BASE

 xen/arch/arm/arm64/domctl.c |  2 +-
 xen/arch/x86/domain.c   |  2 +-
 xen/arch/x86/hvm/vmx/vmx.c  | 25 ---
 xen/arch/x86/mm.c   | 15 ++-
 xen/arch/x86/mm/p2m-pod.c   | 10 ++---
 xen/arch/x86/mm/shadow/common.c |  2 +-
 xen/arch/x86/msi.c  |  2 +-
 xen/arch/x86/numa.c |  2 +-
 xen/arch/x86/pv/dom0_build.c| 25 ++-
 xen/arch/x86/pv/domain.c|  2 +-
 xen/arch/x86/pv/shim.c  |  4 +-
 xen/common/domctl.c |  2 +-
 xen/common/grant_table.c|  4 +-
 xen/common/keyhandler.c |  2 +-
 xen/common/memory.c |  2 +-
 xen/common/page_alloc.c | 78 -
 xen/include/asm-arm/mm.h|  5 ++-
 xen/include/asm-x86/mm.h|  9 ++--
 xen/include/public/memory.h |  4 +-
 xen/include/xen/sched.h | 27 +---
 20 files changed, 143 insertions(+), 81 deletions(-)
---
Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Julien Grall 
Cc: Jun Nakajima 
Cc: Konrad Rzeszutek Wilk 
Cc: "Roger Pau Monné" 
Cc: Stefano Stabellini 
Cc: Tim Deegan 
Cc: Volodymyr Babchuk 
Cc: Wei Liu 
-- 
2.20.1


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 2/8] x86/setup: Fix badpage= handling for memory above HYPERVISOR_VIRT_END

[Julien Grall]

Hi David,

On 01/02/2020 00:32, David Woodhouse wrote:

From: David Woodhouse 

Bad pages are identified by get_platform_badpages() and with badpage=
on the command line.

The boot allocator currently automatically elides these from the regions
passed to it with init_boot_pages(). The xenheap is then initialised
with the pages which are still marked as free by the boot allocator when
end_boot_allocator() is called.

However, any memory above HYPERVISOR_VIRT_END is passed directly to
init_domheap_pages() later in __start_xen(), and the bad page list is
not consulted.

Fix this by marking those pages as PGC_broken in the frametable at the
time end_boot_allocator() runs, and then making init_heap_pages() skip
over any pages which are so marked.

Signed-off-by: David Woodhouse 
---
  xen/common/page_alloc.c | 82 +++--
  1 file changed, 79 insertions(+), 3 deletions(-)

diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 919a270587..3cf478311b 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -1758,6 +1758,18 @@ int query_page_offline(mfn_t mfn, uint32_t *status)
  return 0;
  }
  
+static unsigned long contig_avail_pages(struct page_info *pg, unsigned long max_pages)

+{
+unsigned long i;
+
+for ( i = 0 ; i < max_pages; i++)
+{
+if ( pg[i].count_info & PGC_broken )
+break;
+}
+return i;
+}
+
  /*
   * Hand the specified arbitrary page range to the specified heap zone
   * checking the node_id of the previous page.  If they differ and the
@@ -1799,18 +1811,23 @@ static void init_heap_pages(
  {
  unsigned int nid = phys_to_nid(page_to_maddr(pg+i));
  
+/* If the (first) page is already marked broken, don't add it. */

+if ( pg[i].count_info & PGC_broken )
+continue;
+
  if ( unlikely(!avail[nid]) )
  {
+unsigned long contig_nr_pages = contig_avail_pages(pg + i, 
nr_pages);
  unsigned long s = mfn_x(page_to_mfn(pg + i));
-unsigned long e = mfn_x(mfn_add(page_to_mfn(pg + nr_pages - 1), 
1));
+unsigned long e = mfn_x(mfn_add(page_to_mfn(pg + i + 
contig_nr_pages - 1), 1));
  bool use_tail = (nid == phys_to_nid(pfn_to_paddr(e - 1))) &&
  !(s & ((1UL << MAX_ORDER) - 1)) &&
  (find_first_set_bit(e) <= find_first_set_bit(s));
  unsigned long n;
  
-n = init_node_heap(nid, mfn_x(page_to_mfn(pg + i)), nr_pages - i,

+n = init_node_heap(nid, mfn_x(page_to_mfn(pg + i)), 
contig_nr_pages,
 &use_tail);
-BUG_ON(i + n > nr_pages);
+BUG_ON(n > contig_nr_pages);
  if ( n && !use_tail )
  {
  i += n - 1;
@@ -1846,6 +1863,63 @@ static unsigned long avail_heap_pages(
  return free_pages;
  }
  
+static void mark_bad_pages(void)

+{
+unsigned long bad_spfn, bad_epfn;


I would like to avoid adding more 'pfn' in the code. Per the current 
terminology (see include/xen/mm.h), this would be bad_smfn, bad_emfn.


Ideally, this should also use mfn_t, but I can live without it for now.


+const char *p;
+struct page_info *pg;
+#ifdef CONFIG_X86
+const struct platform_bad_page *badpage;
+unsigned int i, j, array_size;
+
+badpage = get_platform_badpages(&array_size);
+if ( badpage )
+{
+for ( i = 0; i < array_size; i++ )
+{
+for ( j = 0; j < 1UL << badpage->order; j++ )
+{
+if ( mfn_valid(_mfn(badpage->mfn + j)) )
+{
+pg = mfn_to_page(_mfn(badpage->mfn + j));
+pg->count_info |= PGC_broken;
+page_list_add_tail(pg, &page_broken_list);
+}
+}
+}
+}


You are missing the PV shim part here.

But, AFAICT, the only difference with the implementation in 
init_boot_pages() is how we treat the page.


Could you we introduce a common helper taking a callback? The callback 
would be applied on every range range.



+#endif
+
+/* Check new pages against the bad-page list. */
+p = opt_badpage;
+while ( *p != '\0' )
+{
+bad_spfn = simple_strtoul(p, &p, 0);
+bad_epfn = bad_spfn;
+
+if ( *p == '-' )
+{
+p++;
+bad_epfn = simple_strtoul(p, &p, 0);
+if ( bad_epfn < bad_spfn )
+bad_epfn = bad_spfn;
+}
+
+if ( *p == ',' )
+p++;
+else if ( *p != '\0' )
+break;
+
+while ( mfn_valid(_mfn(bad_spfn)) && bad_spfn < bad_epfn )
+{
+pg = mfn_to_page(_mfn(bad_spfn));
+pg->count_info |= PGC_broken;
+page_list_add_tail(pg, &page_broken_list);
+bad_spfn++;
+}
+}
+}
+
  void __init end_boot_allocator(void)
  {
  unsigned int i;
@@ -187

[Xen-devel] [PATCH v9 4/4] x86 / vmx: use a MEMF_no_refcount domheap page for APIC_DEFAULT_PHYS_BASE

[Paul Durrant]

vmx_alloc_vlapic_mapping() currently contains some very odd looking code
that allocates a MEMF_no_owner domheap page and then shares with the guest
as if it were a xenheap page. This then requires vmx_free_vlapic_mapping()
to call a special function in the mm code: free_shared_domheap_page().

By using a MEMF_no_refcount domheap page instead, the odd looking code in
vmx_alloc_vlapic_mapping() can simply use get_page_and_type() to set up a
writable mapping before insertion in the P2M and vmx_free_vlapic_mapping()
can simply release the page using put_page_alloc_ref() followed by
put_page_and_type(). This then allows free_shared_domheap_page() to be
purged.

Signed-off-by: Paul Durrant 
Reviewed-by: Jan Beulich 
Reviewed-by: Kevin Tian 
---
Cc: Jun Nakajima 
Cc: Andrew Cooper 
Cc: Wei Liu 
Cc: "Roger Pau Monné" 

v4:
 - Use a MEMF_no_refcount page rather than a 'normal' page

v2:
 - Set an initial value for max_pages rather than avoiding the check in
   assign_pages()
 - Make domain_destroy() optional
---
 xen/arch/x86/hvm/vmx/vmx.c | 21 ++---
 xen/arch/x86/mm.c  | 10 --
 xen/include/asm-x86/mm.h   |  2 --
 3 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 606f3dc2eb..7423d2421b 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3028,12 +3028,22 @@ static int vmx_alloc_vlapic_mapping(struct domain *d)
 if ( !cpu_has_vmx_virtualize_apic_accesses )
 return 0;
 
-pg = alloc_domheap_page(d, MEMF_no_owner);
+pg = alloc_domheap_page(d, MEMF_no_refcount);
 if ( !pg )
 return -ENOMEM;
+
+if ( !get_page_and_type(pg, d, PGT_writable_page) )
+{
+/*
+ * The domain can't possibly know about this page yet, so failure
+ * here is a clear indication of something fishy going on.
+ */
+domain_crash(d);
+return -ENODATA;
+}
+
 mfn = page_to_mfn(pg);
 clear_domain_page(mfn);
-share_xen_page_with_guest(pg, d, SHARE_rw);
 d->arch.hvm.vmx.apic_access_mfn = mfn;
 
 return set_mmio_p2m_entry(d, paddr_to_pfn(APIC_DEFAULT_PHYS_BASE), mfn,
@@ -3047,7 +3057,12 @@ static void vmx_free_vlapic_mapping(struct domain *d)
 
 d->arch.hvm.vmx.apic_access_mfn = _mfn(0);
 if ( !mfn_eq(mfn, _mfn(0)) )
-free_shared_domheap_page(mfn_to_page(mfn));
+{
+struct page_info *pg = mfn_to_page(mfn);
+
+put_page_alloc_ref(pg);
+put_page_and_type(pg);
+}
 }
 
 static void vmx_install_vlapic_mapping(struct vcpu *v)
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index fd134edcde..1e49bb0156 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -496,16 +496,6 @@ void share_xen_page_with_guest(struct page_info *page, 
struct domain *d,
 spin_unlock(&d->page_alloc_lock);
 }
 
-void free_shared_domheap_page(struct page_info *page)
-{
-put_page_alloc_ref(page);
-if ( !test_and_clear_bit(_PGC_xen_heap, &page->count_info) )
-ASSERT_UNREACHABLE();
-page->u.inuse.type_info = 0;
-page_set_owner(page, NULL);
-free_domheap_page(page);
-}
-
 void make_cr3(struct vcpu *v, mfn_t mfn)
 {
 struct domain *d = v->domain;
diff --git a/xen/include/asm-x86/mm.h b/xen/include/asm-x86/mm.h
index 06d64d494d..fafb3af46d 100644
--- a/xen/include/asm-x86/mm.h
+++ b/xen/include/asm-x86/mm.h
@@ -320,8 +320,6 @@ struct page_info
 
 #define maddr_get_owner(ma)   (page_get_owner(maddr_to_page((ma
 
-extern void free_shared_domheap_page(struct page_info *page);
-
 #define frame_table ((struct page_info *)FRAMETABLE_VIRT_START)
 extern unsigned long max_page;
 extern unsigned long total_pages;
-- 
2.20.1


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v6 05/11] x86/hyperv: setup hypercall page

[Jan Beulich]

On 03.02.2020 11:39, Durrant, Paul wrote:
>> -Original Message-
>> From: Wei Liu 
>> Sent: 31 January 2020 17:57
>> To: Xen Development List 
>> Cc: Durrant, Paul ; Michael Kelley
>> ; Wei Liu ; Wei Liu
>> ; Jan Beulich ; Andrew Cooper
>> ; Roger Pau Monné 
>> Subject: Re: [PATCH v6 05/11] x86/hyperv: setup hypercall page
>>
>> (Note to self)
>>
>> On Fri, Jan 31, 2020 at 05:49:24PM +, Wei Liu wrote:
>> [...]
>>> +static uint64_t generate_guest_id(void)
>>> +{
>>> +union hv_guest_os_id id;
>>> +
>>
>>id.raw = 0;
> 
> Or just use a C99 initializer to set things up. A bit neater IMO.

If you mean this for ...

>>> +id.vendor = HV_XEN_VENDOR_ID;
>>> +id.major = xen_major_version();
>>> +id.minor = xen_minor_version();

... these three fields, then this won't work with rather old gcc
we still permit to be used. Using { .raw = 0 } would work afaict.

Jan

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 6/8] x86/setup: move vm_init() before end_boot_allocator()

[Xia, Hongyan]

On Sat, 2020-02-01 at 00:33 +, David Woodhouse wrote:
> From: David Woodhouse 
> 
> We would like to be able to use vmap() to map the live update data,
> and
> we need to do a first pass of the live update data before we prime
> the
> heap because we need to know which pages need to be preserved.
> 
> The warning about ACPI code can be dropped, since that problem no
> longer
> exists when things are done in this order.
> 
> Signed-off-by: David Woodhouse 
> ---
>  xen/arch/x86/setup.c | 8 ++--
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index 2677f127b9..5f68a1308f 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -1489,6 +1489,8 @@ void __init noreturn __start_xen(unsigned long
> mbi_p)
>  
>  numa_initmem_init(0, raw_max_page);
>  
> +vm_init();
> +
>  if ( max_page - 1 > virt_to_mfn(HYPERVISOR_VIRT_END - 1) )
>  {
>  unsigned long limit = virt_to_mfn(HYPERVISOR_VIRT_END - 1);
> @@ -1519,12 +1521,6 @@ void __init noreturn __start_xen(unsigned long
> mbi_p)
>  end_boot_allocator();
>  
>  system_state = SYS_STATE_boot;
> -/*
> - * No calls involving ACPI code should go between the setting of
> - * SYS_STATE_boot and vm_init() (or else
> acpi_os_{,un}map_memory()
> - * will break).
> - */
> -vm_init();
>  
>  console_init_ring();
>  vesa_init();

Is there any problem to move vm_init() even earlier than this, like
right after init_frametable()? ACPI and NUMA functions need a couple of
things permanently mapped. Without the directmap (directmap removal
series), xenheap and vmap at this stage, I can only map memory to the
directmap region which is really hacky. I would like to use vmap at
this stage so hopefully vmap can be ready very early.

Hongyan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] docs/misc: xen-command-line: fix parameters in sample serial configuration

[Jan Beulich]

On 01.02.2020 02:20, Sarah Newman wrote:
> The names of the serial parameters use hyphens, not underscores.
> 
> Signed-off-by: Sarah Newman 

Reviewed-by: Jan Beulich 


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v6 05/11] x86/hyperv: setup hypercall page

[Durrant, Paul]

> -Original Message-
> From: Jan Beulich 
> Sent: 03 February 2020 11:06
> To: Durrant, Paul 
> Cc: Wei Liu ; Xen Development List  de...@lists.xenproject.org>; Michael Kelley ; Wei
> Liu ; Andrew Cooper ;
> Roger Pau Monné 
> Subject: Re: [PATCH v6 05/11] x86/hyperv: setup hypercall page
> 
> On 03.02.2020 11:39, Durrant, Paul wrote:
> >> -Original Message-
> >> From: Wei Liu 
> >> Sent: 31 January 2020 17:57
> >> To: Xen Development List 
> >> Cc: Durrant, Paul ; Michael Kelley
> >> ; Wei Liu ; Wei Liu
> >> ; Jan Beulich ; Andrew Cooper
> >> ; Roger Pau Monné 
> >> Subject: Re: [PATCH v6 05/11] x86/hyperv: setup hypercall page
> >>
> >> (Note to self)
> >>
> >> On Fri, Jan 31, 2020 at 05:49:24PM +, Wei Liu wrote:
> >> [...]
> >>> +static uint64_t generate_guest_id(void)
> >>> +{
> >>> +union hv_guest_os_id id;
> >>> +
> >>
> >>id.raw = 0;
> >
> > Or just use a C99 initializer to set things up. A bit neater IMO.
> 
> If you mean this for ...
> 
> >>> +id.vendor = HV_XEN_VENDOR_ID;
> >>> +id.major = xen_major_version();
> >>> +id.minor = xen_minor_version();
> 
> ... these three fields, then this won't work with rather old gcc
> we still permit to be used. Using { .raw = 0 } would work afaict.
> 

Not even { .vendor = HV_XEN_VENDOR_ID } ?

  Paul

> Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v3] ns16550: Add ACPI support for ARM only

[Wei Xu]

Parse the ACPI SPCR table and initialize the 16550 compatible serial port
for ARM only. Currently we only support one UART on ARM. Some fields
which we do not care yet on ARM are ignored.

Signed-off-by: Wei Xu 

---
Changes in v3:
- address the code style comments from Jan
- use container_of to do cast
- list all fields we ignored
- check the console redirection is disabled or not before init the uart
- init the uart io_size and width via spcr->serial_port

Changes in v2:
- improve commit message
- remove the spcr initialization
- add comments for the uart initialization and configuration
- adjust the code style issue
- limit the code only built on ACPI and ARM
---
 xen/drivers/char/ns16550.c | 75 ++
 1 file changed, 75 insertions(+)

diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index aa87c57..741b510 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -1620,6 +1620,81 @@ DT_DEVICE_START(ns16550, "NS16550 UART", DEVICE_SERIAL)
 DT_DEVICE_END

 #endif /* HAS_DEVICE_TREE */
+
+#if defined(CONFIG_ACPI) && defined(CONFIG_ARM)
+#include 
+
+static int __init ns16550_acpi_uart_init(const void *data)
+{
+struct acpi_table_header *table;
+struct acpi_table_spcr *spcr;
+acpi_status status;
+/*
+ * Same as the DT part.
+ * Only support one UART on ARM which happen to be ns16550_com[0].
+ */
+struct ns16550 *uart = &ns16550_com[0];
+
+status = acpi_get_table(ACPI_SIG_SPCR, 0, &table);
+if ( ACPI_FAILURE(status) )
+{
+printk("ns16550: Failed to get SPCR table\n");
+return -EINVAL;
+}
+
+spcr = container_of(table, struct acpi_table_spcr, header);
+
+/*
+ * The serial port address may be 0 for example
+ * if the console redirection is disabled.
+ */
+if ( unlikely(!spcr->serial_port.address) )
+{
+printk("ns16550: the serial port address is invalid\n");
+return -EINVAL;
+}
+
+ns16550_init_common(uart);
+
+/*
+ * The baud rate is pre-configured by the firmware.
+ * And currently the ACPI part is only targeting ARM so the following
+ * fields pc_interrupt, pci_device_id, pci_vendor_id, pci_bus, pci_device,
+ * pci_function, pci_flags, pci_segment and flow_control which we do not
+ * care yet are ignored.
+ */
+uart->baud = BAUD_AUTO;
+uart->data_bits = 8;
+uart->parity = spcr->parity;
+uart->stop_bits = spcr->stop_bits;
+uart->io_base = spcr->serial_port.address;
+uart->io_size = spcr->serial_port.bit_width;
+uart->reg_shift = spcr->serial_port.bit_offset;
+uart->reg_width = spcr->serial_port.access_width;
+
+/* The trigger/polarity information is not available in spcr. */
+irq_set_type(spcr->interrupt, IRQ_TYPE_LEVEL_HIGH);
+uart->irq = spcr->interrupt;
+
+uart->vuart.base_addr = uart->io_base;
+uart->vuart.size = uart->io_size;
+uart->vuart.data_off = UART_THR << uart->reg_shift;
+uart->vuart.status_off = UART_LSR << uart->reg_shift;
+uart->vuart.status = UART_LSR_THRE | UART_LSR_TEMT;
+
+/* Register with generic serial driver. */
+serial_register_uart(SERHND_DTUART, &ns16550_driver, uart);
+
+return 0;
+}
+
+ACPI_DEVICE_START(ans16550, "NS16550 UART", DEVICE_SERIAL)
+.class_type = ACPI_DBG2_16550_COMPATIBLE,
+.init = ns16550_acpi_uart_init,
+ACPI_DEVICE_END
+
+#endif /* CONFIG_ACPI && CONFIG_ARM */
+
 /*
  * Local variables:
  * mode: C
-- 
2.8.1


.



___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v6 10/11] x86: move viridian_page_msr to hyperv-tlfs.h

[Durrant, Paul]

> -Original Message-
> From: Wei Liu  On Behalf Of Wei Liu
> Sent: 31 January 2020 17:49
> To: Xen Development List 
> Cc: Durrant, Paul ; Michael Kelley
> ; Wei Liu ; Wei Liu
> ; Jan Beulich ; Andrew Cooper
> ; Roger Pau Monné 
> Subject: [PATCH v6 10/11] x86: move viridian_page_msr to hyperv-tlfs.h
> 
> And rename it to hv_vp_assist_page_msr.
> 
> No functional change.
> 
> Signed-off-by: Wei Liu 

Reviewed-by: Paul Durrant 

> ---
>  xen/arch/x86/hvm/viridian/viridian.c|  2 +-
>  xen/include/asm-x86/guest/hyperv-tlfs.h | 11 +++
>  xen/include/asm-x86/hvm/viridian.h  | 15 ++-
>  3 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/viridian/viridian.c
> b/xen/arch/x86/hvm/viridian/viridian.c
> index 44c8e6cac6..9a6cafcb62 100644
> --- a/xen/arch/x86/hvm/viridian/viridian.c
> +++ b/xen/arch/x86/hvm/viridian/viridian.c
> @@ -230,7 +230,7 @@ static void dump_guest_os_id(const struct domain *d)
> 
>  static void dump_hypercall(const struct domain *d)
>  {
> -const union viridian_page_msr *hg;
> +const union hv_vp_assist_page_msr *hg;
> 
>  hg = &d->arch.hvm.viridian->hypercall_gpa;
> 
> diff --git a/xen/include/asm-x86/guest/hyperv-tlfs.h b/xen/include/asm-
> x86/guest/hyperv-tlfs.h
> index 07db57b55f..0a0f3398c1 100644
> --- a/xen/include/asm-x86/guest/hyperv-tlfs.h
> +++ b/xen/include/asm-x86/guest/hyperv-tlfs.h
> @@ -558,6 +558,17 @@ struct hv_nested_enlightenments_control {
>   } hypercallControls;
>  };
> 
> +union hv_vp_assist_page_msr
> +{
> +uint64_t raw;
> +struct
> +{
> +uint64_t enabled:1;
> +uint64_t reserved_preserved:11;
> +uint64_t pfn:48;
> +};
> +};
> +
>  /* Define virtual processor assist page structure. */
>  struct hv_vp_assist_page {
>   __u32 apic_assist;
> diff --git a/xen/include/asm-x86/hvm/viridian.h b/xen/include/asm-
> x86/hvm/viridian.h
> index d9138562e6..844e56b38f 100644
> --- a/xen/include/asm-x86/hvm/viridian.h
> +++ b/xen/include/asm-x86/hvm/viridian.h
> @@ -11,20 +11,9 @@
> 
>  #include 
> 
> -union viridian_page_msr
> -{
> -uint64_t raw;
> -struct
> -{
> -uint64_t enabled:1;
> -uint64_t reserved_preserved:11;
> -uint64_t pfn:48;
> -};
> -};
> -
>  struct viridian_page
>  {
> -union viridian_page_msr msr;
> +union hv_vp_assist_page_msr msr;
>  void *ptr;
>  };
> 
> @@ -70,7 +59,7 @@ struct viridian_time_ref_count
>  struct viridian_domain
>  {
>  union hv_guest_os_id guest_os_id;
> -union viridian_page_msr hypercall_gpa;
> +union hv_vp_assist_page_msr hypercall_gpa;
>  struct viridian_time_ref_count time_ref_count;
>  struct viridian_page reference_tsc;
>  };
> --
> 2.20.1


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v6 11/11] x86/hyperv: setup VP assist page

[Durrant, Paul]

> -Original Message-
> From: Wei Liu  On Behalf Of Wei Liu
> Sent: 31 January 2020 17:50
> To: Xen Development List 
> Cc: Durrant, Paul ; Michael Kelley
> ; Wei Liu ; Wei Liu
> ; Jan Beulich ; Andrew Cooper
> ; Roger Pau Monné 
> Subject: [PATCH v6 11/11] x86/hyperv: setup VP assist page
> 
> VP assist page is rather important as we need to toggle some bits in it
> for efficient nested virtualisation.
> 
> Signed-off-by: Wei Liu 

Reviewed-by: Paul Durrant 

> ---
> v6:
> 1. Use hv_vp_assist_page_msr
> 2. Make code shorter
> 3. Preserve rsvdP fields
> 
> v5:
> 1. Deal with error properly instead of always panicking
> 2. Swap percpu variables declarations' location
> 
> v4:
> 1. Use private.h
> 2. Prevent leak
> 
> v3:
> 1. Use xenheap page
> 2. Drop set_vp_assist
> 
> v2:
> 1. Use HV_HYP_PAGE_SHIFT instead
> ---
>  xen/arch/x86/guest/hyperv/hyperv.c  | 37 -
>  xen/arch/x86/guest/hyperv/private.h |  1 +
>  2 files changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/x86/guest/hyperv/hyperv.c
> b/xen/arch/x86/guest/hyperv/hyperv.c
> index 6dac3bfceb..75fb329d4d 100644
> --- a/xen/arch/x86/guest/hyperv/hyperv.c
> +++ b/xen/arch/x86/guest/hyperv/hyperv.c
> @@ -31,6 +31,7 @@
> 
>  struct ms_hyperv_info __read_mostly ms_hyperv;
>  DEFINE_PER_CPU_READ_MOSTLY(void *, hv_input_page);
> +DEFINE_PER_CPU_READ_MOSTLY(void *, hv_vp_assist);
>  DEFINE_PER_CPU_READ_MOSTLY(unsigned int, hv_vp_index);
> 
>  static uint64_t generate_guest_id(void)
> @@ -155,17 +156,51 @@ static int setup_hypercall_pcpu_arg(void)
>  return 0;
>  }
> 
> +static int setup_vp_assist(void)
> +{
> +union hv_vp_assist_page_msr msr;
> +
> +if ( !this_cpu(hv_vp_assist) )
> +{
> +this_cpu(hv_vp_assist) = alloc_xenheap_page();
> +if ( !this_cpu(hv_vp_assist) )
> +{
> +printk("CPU%u: Failed to allocate vp_assist page\n",
> +   smp_processor_id());
> +return -ENOMEM;
> +}
> +
> +clear_page(this_cpu(hv_vp_assist));
> +}
> +
> +rdmsrl(HV_X64_MSR_VP_ASSIST_PAGE, msr.raw);
> +msr.pfn = virt_to_mfn(this_cpu(hv_vp_assist));
> +msr.enabled = 1;
> +wrmsrl(HV_X64_MSR_VP_ASSIST_PAGE, msr.raw);
> +
> +return 0;
> +}
> +
>  static void __init setup(void)
>  {
>  setup_hypercall_page();
> 
>  if ( setup_hypercall_pcpu_arg() )
>  panic("Hyper-V hypercall percpu arg setup failed\n");
> +
> +if ( setup_vp_assist() )
> +panic("VP assist page setup failed\n");
>  }
> 
>  static int ap_setup(void)
>  {
> -return setup_hypercall_pcpu_arg();
> +int rc;
> +
> +rc = setup_hypercall_pcpu_arg();
> +if ( rc )
> +return rc;
> +
> +return setup_vp_assist();
>  }
> 
>  static void __init e820_fixup(struct e820map *e820)
> diff --git a/xen/arch/x86/guest/hyperv/private.h
> b/xen/arch/x86/guest/hyperv/private.h
> index d1765d4f23..956eff831f 100644
> --- a/xen/arch/x86/guest/hyperv/private.h
> +++ b/xen/arch/x86/guest/hyperv/private.h
> @@ -25,6 +25,7 @@
>  #include 
> 
>  DECLARE_PER_CPU(void *, hv_input_page);
> +DECLARE_PER_CPU(void *, hv_vp_assist);
>  DECLARE_PER_CPU(unsigned int, hv_vp_index);
> 
>  #endif /* __XEN_HYPERV_PRIVIATE_H__  */
> --
> 2.20.1


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 02/12] xen/build: Use obj-y += subdir/ instead of subdir-y

[Anthony PERARD]

On Fri, Jan 31, 2020 at 09:35:16AM +0100, Jan Beulich wrote:
> Alternatively, to retain this "latching" effect, how about
> 
> subdir-y := $(subdir-y) $(filter %/, $(obj-y))

Sounds good.

Thanks,

-- 
Anthony PERARD

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v6 05/11] x86/hyperv: setup hypercall page

[Jan Beulich]

On 03.02.2020 12:21, Durrant, Paul wrote:
>> -Original Message-
>> From: Jan Beulich 
>> Sent: 03 February 2020 11:06
>> To: Durrant, Paul 
>> Cc: Wei Liu ; Xen Development List > de...@lists.xenproject.org>; Michael Kelley ; Wei
>> Liu ; Andrew Cooper ;
>> Roger Pau Monné 
>> Subject: Re: [PATCH v6 05/11] x86/hyperv: setup hypercall page
>>
>> On 03.02.2020 11:39, Durrant, Paul wrote:
 -Original Message-
 From: Wei Liu 
 Sent: 31 January 2020 17:57
 To: Xen Development List 
 Cc: Durrant, Paul ; Michael Kelley
 ; Wei Liu ; Wei Liu
 ; Jan Beulich ; Andrew Cooper
 ; Roger Pau Monné 
 Subject: Re: [PATCH v6 05/11] x86/hyperv: setup hypercall page

 (Note to self)

 On Fri, Jan 31, 2020 at 05:49:24PM +, Wei Liu wrote:
 [...]
> +static uint64_t generate_guest_id(void)
> +{
> +union hv_guest_os_id id;
> +

id.raw = 0;
>>>
>>> Or just use a C99 initializer to set things up. A bit neater IMO.
>>
>> If you mean this for ...
>>
> +id.vendor = HV_XEN_VENDOR_ID;
> +id.major = xen_major_version();
> +id.minor = xen_minor_version();
>>
>> ... these three fields, then this won't work with rather old gcc
>> we still permit to be used. Using { .raw = 0 } would work afaict.
>>
> 
> Not even { .vendor = HV_XEN_VENDOR_ID } ?

No, because of it being part of an unnamed (struct) member of
the union.

Jan

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v6 05/11] x86/hyperv: setup hypercall page

[Durrant, Paul]

> -Original Message-
> From: Jan Beulich 
> Sent: 03 February 2020 11:34
> To: Durrant, Paul 
> Cc: Wei Liu ; Xen Development List  de...@lists.xenproject.org>; Michael Kelley ; Wei
> Liu ; Andrew Cooper ;
> Roger Pau Monné 
> Subject: Re: [PATCH v6 05/11] x86/hyperv: setup hypercall page
> 
> On 03.02.2020 12:21, Durrant, Paul wrote:
> >> -Original Message-
> >> From: Jan Beulich 
> >> Sent: 03 February 2020 11:06
> >> To: Durrant, Paul 
> >> Cc: Wei Liu ; Xen Development List  >> de...@lists.xenproject.org>; Michael Kelley ;
> Wei
> >> Liu ; Andrew Cooper ;
> >> Roger Pau Monné 
> >> Subject: Re: [PATCH v6 05/11] x86/hyperv: setup hypercall page
> >>
> >> On 03.02.2020 11:39, Durrant, Paul wrote:
>  -Original Message-
>  From: Wei Liu 
>  Sent: 31 January 2020 17:57
>  To: Xen Development List 
>  Cc: Durrant, Paul ; Michael Kelley
>  ; Wei Liu ; Wei Liu
>  ; Jan Beulich ; Andrew Cooper
>  ; Roger Pau Monné 
>  Subject: Re: [PATCH v6 05/11] x86/hyperv: setup hypercall page
> 
>  (Note to self)
> 
>  On Fri, Jan 31, 2020 at 05:49:24PM +, Wei Liu wrote:
>  [...]
> > +static uint64_t generate_guest_id(void)
> > +{
> > +union hv_guest_os_id id;
> > +
> 
> id.raw = 0;
> >>>
> >>> Or just use a C99 initializer to set things up. A bit neater IMO.
> >>
> >> If you mean this for ...
> >>
> > +id.vendor = HV_XEN_VENDOR_ID;
> > +id.major = xen_major_version();
> > +id.minor = xen_minor_version();
> >>
> >> ... these three fields, then this won't work with rather old gcc
> >> we still permit to be used. Using { .raw = 0 } would work afaict.
> >>
> >
> > Not even { .vendor = HV_XEN_VENDOR_ID } ?
> 
> No, because of it being part of an unnamed (struct) member of
> the union.

Ok... shame. Presumably an empty initializer - {} -  would be ok?

  Paul

> 
> Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v6 05/11] x86/hyperv: setup hypercall page

[Jan Beulich]

On 03.02.2020 12:37, Durrant, Paul wrote:
>> -Original Message-
>> From: Jan Beulich 
>> Sent: 03 February 2020 11:34
>> To: Durrant, Paul 
>> Cc: Wei Liu ; Xen Development List > de...@lists.xenproject.org>; Michael Kelley ; Wei
>> Liu ; Andrew Cooper ;
>> Roger Pau Monné 
>> Subject: Re: [PATCH v6 05/11] x86/hyperv: setup hypercall page
>>
>> On 03.02.2020 12:21, Durrant, Paul wrote:
 -Original Message-
 From: Jan Beulich 
 Sent: 03 February 2020 11:06
 To: Durrant, Paul 
 Cc: Wei Liu ; Xen Development List >>> de...@lists.xenproject.org>; Michael Kelley ;
>> Wei
 Liu ; Andrew Cooper ;
 Roger Pau Monné 
 Subject: Re: [PATCH v6 05/11] x86/hyperv: setup hypercall page

 On 03.02.2020 11:39, Durrant, Paul wrote:
>> -Original Message-
>> From: Wei Liu 
>> Sent: 31 January 2020 17:57
>> To: Xen Development List 
>> Cc: Durrant, Paul ; Michael Kelley
>> ; Wei Liu ; Wei Liu
>> ; Jan Beulich ; Andrew Cooper
>> ; Roger Pau Monné 
>> Subject: Re: [PATCH v6 05/11] x86/hyperv: setup hypercall page
>>
>> (Note to self)
>>
>> On Fri, Jan 31, 2020 at 05:49:24PM +, Wei Liu wrote:
>> [...]
>>> +static uint64_t generate_guest_id(void)
>>> +{
>>> +union hv_guest_os_id id;
>>> +
>>
>>id.raw = 0;
>
> Or just use a C99 initializer to set things up. A bit neater IMO.

 If you mean this for ...

>>> +id.vendor = HV_XEN_VENDOR_ID;
>>> +id.major = xen_major_version();
>>> +id.minor = xen_minor_version();

 ... these three fields, then this won't work with rather old gcc
 we still permit to be used. Using { .raw = 0 } would work afaict.

>>>
>>> Not even { .vendor = HV_XEN_VENDOR_ID } ?
>>
>> No, because of it being part of an unnamed (struct) member of
>> the union.
> 
> Ok... shame. Presumably an empty initializer - {} -  would be ok?

I think so, yes. I understand you'd like this better than
{ .raw = 0 } ?

Jan

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v9 2/4] add a domain_tot_pages() helper function

[Jan Beulich]

On 03.02.2020 11:56, Paul Durrant wrote:
> This patch adds a new domain_tot_pages() inline helper function into
> sched.h, which will be needed by a subsequent patch.
> 
> No functional change.
> 
> NOTE: While modifying the comment for 'tot_pages' in sched.h this patch
>   makes some cosmetic fixes to surrounding comments.
> 
> Suggested-by: Jan Beulich 
> Signed-off-by: Paul Durrant 

Reviewed-by: Jan Beulich 


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v6 05/11] x86/hyperv: setup hypercall page

[Durrant, Paul]

> -Original Message-
> From: Jan Beulich 
> Sent: 03 February 2020 11:39
> To: Durrant, Paul 
> Cc: Wei Liu ; Xen Development List  de...@lists.xenproject.org>; Michael Kelley ; Wei
> Liu ; Andrew Cooper ;
> Roger Pau Monné 
> Subject: Re: [PATCH v6 05/11] x86/hyperv: setup hypercall page
> 
> On 03.02.2020 12:37, Durrant, Paul wrote:
> >> -Original Message-
> >> From: Jan Beulich 
> >> Sent: 03 February 2020 11:34
> >> To: Durrant, Paul 
> >> Cc: Wei Liu ; Xen Development List  >> de...@lists.xenproject.org>; Michael Kelley ;
> Wei
> >> Liu ; Andrew Cooper ;
> >> Roger Pau Monné 
> >> Subject: Re: [PATCH v6 05/11] x86/hyperv: setup hypercall page
> >>
> >> On 03.02.2020 12:21, Durrant, Paul wrote:
>  -Original Message-
>  From: Jan Beulich 
>  Sent: 03 February 2020 11:06
>  To: Durrant, Paul 
>  Cc: Wei Liu ; Xen Development List   de...@lists.xenproject.org>; Michael Kelley ;
> >> Wei
>  Liu ; Andrew Cooper ;
>  Roger Pau Monné 
>  Subject: Re: [PATCH v6 05/11] x86/hyperv: setup hypercall page
> 
>  On 03.02.2020 11:39, Durrant, Paul wrote:
> >> -Original Message-
> >> From: Wei Liu 
> >> Sent: 31 January 2020 17:57
> >> To: Xen Development List 
> >> Cc: Durrant, Paul ; Michael Kelley
> >> ; Wei Liu ; Wei Liu
> >> ; Jan Beulich ; Andrew Cooper
> >> ; Roger Pau Monné 
> >> Subject: Re: [PATCH v6 05/11] x86/hyperv: setup hypercall page
> >>
> >> (Note to self)
> >>
> >> On Fri, Jan 31, 2020 at 05:49:24PM +, Wei Liu wrote:
> >> [...]
> >>> +static uint64_t generate_guest_id(void)
> >>> +{
> >>> +union hv_guest_os_id id;
> >>> +
> >>
> >>id.raw = 0;
> >
> > Or just use a C99 initializer to set things up. A bit neater IMO.
> 
>  If you mean this for ...
> 
> >>> +id.vendor = HV_XEN_VENDOR_ID;
> >>> +id.major = xen_major_version();
> >>> +id.minor = xen_minor_version();
> 
>  ... these three fields, then this won't work with rather old gcc
>  we still permit to be used. Using { .raw = 0 } would work afaict.
> 
> >>>
> >>> Not even { .vendor = HV_XEN_VENDOR_ID } ?
> >>
> >> No, because of it being part of an unnamed (struct) member of
> >> the union.
> >
> > Ok... shame. Presumably an empty initializer - {} -  would be ok?
> 
> I think so, yes. I understand you'd like this better than
> { .raw = 0 } ?
> 

Yes. In general, using a c99 initializer to explicitly set something to 0 seems 
a bit redundant to me.

  Paul

> Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 09/12] xen/build: include include/config/auto.conf in main Makefile

[Anthony PERARD]

On Thu, Jan 30, 2020 at 02:06:18PM +0100, Jan Beulich wrote:
> On 17.01.2020 11:53, Anthony PERARD wrote:
> > --- a/xen/Makefile
> > +++ b/xen/Makefile
> > @@ -49,7 +49,71 @@ default: build
> >  .PHONY: dist
> >  dist: install
> >  
> > -build install:: include/config/auto.conf
> > +
> > +ifndef root-make-done
> > +# section to run before calling Rules.mk, but only once.
> > +#
> > +# To make sure we do not include .config for any of the *config targets
> > +# catch them early, and hand them over to tools/kconfig/Makefile
> > +
> > +clean-targets := %clean
> > +no-dot-config-targets := $(clean-targets) \
> > +uninstall debug cloc \
> > +cscope TAGS tags MAP gtags \
> > +xenversion
> > +
> > +config-build   :=
> 
> Is this actually needed? While correct (afaict) together with the
> ifdef further down, I find this aspect of make behavior a little
> confusing, and hence it would seem slightly better if there was
> no empty definition of this symbol.

That's actually a very recent change in Linux source code. They used to
use ifeq($(config-build),1) and ifeq($(config-build),0). I can certainly
change back to use ifeq instead of ifdef.

> > +need-config:= 1
> 
> Here and below, would it be possible to use y instead of 1, to
> match how "true" gets expressed in various places elsewhere?
> Or would there again be deviation-from-Linux concerns?

It's probably fine to use "y". I don't think it matter, we need to make
quite a lot of changes compare to Linux anyway. I'll use "n" for the
negative.

> > +ifneq ($(filter $(no-dot-config-targets), $(MAKECMDGOALS)),)
> > +   ifeq ($(filter-out $(no-dot-config-targets), $(MAKECMDGOALS)),)
> > +   need-config :=
> > +   endif
> > +endif
> > +
> > +ifneq ($(filter config %config,$(MAKECMDGOALS)),)
> 
> Just $(filter %config, ...) suffices here, afaict, similar to
> above "%clean" also being used to cover "clean".

Yes, I'll remove the extra "config".

> > +   config-build := 1
> > +endif
> > +
> > +export root-make-done := 1
> > +endif # root-make-done
> > +
> > +ifdef config-build
> > +# 
> > ===
> > +# *config targets only - make sure prerequisites are updated, and descend
> > +# in tools/kconfig to make the *config target
> > +
> > +config: FORCE
> > +   $(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) 
> > SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" $@
> > +
> > +# Config.mk tries to include .config file, don't try to remake it
> > +%/.config: ;
> 
> This didn't exist before - why is it needed all of the sudden?

It's because I'm introducing a new target "%config". So when make
"-include $(XEN_ROOT)/.config" (as found in Config.mk) it check if the
file needs to be rebuilt, and find %config and thus run kconfig to build
.config.

Currently, Makefile list all the targets that needs to be built with
kconfig.

> > +%config: FORCE
> > +   $(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) 
> > SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" $@
> > +
> > +else # !config-build
> > +
> > +ifdef need-config
> > +include include/config/auto.conf
> > +# Read in dependencies to all Kconfig* files, make sure to run syncconfig 
> > if
> > +# changes are detected.
> > +include include/config/auto.conf.cmd
> > +
> > +# Allow people to just run `make` as before and not force them to configure
> > +$(KCONFIG_CONFIG):
> > +   $(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) 
> > SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" defconfig
> > +
> > +# The actual configuration files used during the build are stored in
> > +# include/generated/ and include/config/. Update them if .config is newer 
> > than
> > +# include/config/auto.conf (which mirrors .config).
> > +#
> > +# This exploits the 'multi-target pattern rule' trick.
> > +# The syncconfig should be executed only once to make all the targets.
> > +%/auto.conf %/auto.conf.cmd: $(KCONFIG_CONFIG)
> > +   $(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) 
> > SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" syncconfig
> 
> Previously the target pattern was include/config/%.conf. Is there a
> particular reason for the switch?

That change was needed in Linux because the full target is:
%/auto.conf %/auto.conf.cmd %/tristate.conf:
But since we don't need tristate.conf in Xen, I can go back to what we have.

Thanks,

-- 
Anthony PERARD

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 1/7] x86/tlb: fix NEED_FLUSH return type

[Jan Beulich]

On 28.01.2020 15:17, Wei Liu wrote:
> On Mon, Jan 27, 2020 at 07:11:09PM +0100, Roger Pau Monne wrote:
>> The returned type wants to be bool instead of int.
>>
>> No functional change intended.
>>
>> Signed-off-by: Roger Pau Monné 
> 
> Reviewed-by: Wei Liu 

Acked-by: Jan Beulich 



___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v4 1/7] SVM: drop asm/hvm/emulate.h inclusion from vmcb.h

[Durrant, Paul]

> -Original Message-
> From: Xen-devel  On Behalf Of Jan
> Beulich
> Sent: 31 January 2020 16:42
> To: xen-devel@lists.xenproject.org
> Cc: Petre Pircalabu ; Kevin Tian
> ; Tamas K Lengyel ; Wei Liu
> ; Paul Durrant ; George Dunlap
> ; Andrew Cooper ;
> Tim Deegan ; Jun Nakajima ; Alexandru
> Isaila ; Roger Pau Monné 
> Subject: [Xen-devel] [PATCH v4 1/7] SVM: drop asm/hvm/emulate.h inclusion
> from vmcb.h
> 
> It's not needed there and introduces a needless, almost global
> dependency. Include the file (or in some cases just xen/err.h) where
> actually needed, or - in one case - simply forward-declare a struct. In
> microcode*.c take the opportunity and also re-order a few other
> #include-s.
> 
> Signed-off-by: Jan Beulich 

Reviewed-by: Paul Durrant 

> ---
> v4: New.
> 
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -55,6 +55,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> --- a/xen/arch/x86/hvm/ioreq.c
> +++ b/xen/arch/x86/hvm/ioreq.c
> @@ -28,6 +28,7 @@
>  #include 
>  #include 
> 
> +#include 
>  #include 
>  #include 
>  #include 
> --- a/xen/arch/x86/hvm/svm/emulate.c
> +++ b/xen/arch/x86/hvm/svm/emulate.c
> @@ -20,6 +20,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> --- a/xen/arch/x86/hvm/vm_event.c
> +++ b/xen/arch/x86/hvm/vm_event.c
> @@ -22,6 +22,7 @@
> 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
> 
> --- a/xen/arch/x86/microcode.c
> +++ b/xen/arch/x86/microcode.c
> @@ -22,9 +22,10 @@
>   */
> 
>  #include 
> -#include 
> -#include 
> +#include 
>  #include 
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
> --- a/xen/arch/x86/microcode_amd.c
> +++ b/xen/arch/x86/microcode_amd.c
> @@ -14,9 +14,10 @@
>   *  License version 2. See file COPYING for details.
>   */
> 
> -#include 
> -#include 
> +#include 
>  #include 
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
> --- a/xen/arch/x86/microcode_intel.c
> +++ b/xen/arch/x86/microcode_intel.c
> @@ -21,9 +21,10 @@
>   * 2 of the License, or (at your option) any later version.
>   */
> 
> -#include 
> -#include 
> +#include 
>  #include 
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
> --- a/xen/arch/x86/mm/shadow/hvm.c
> +++ b/xen/arch/x86/mm/shadow/hvm.c
> @@ -28,6 +28,7 @@
>  #include 
> 
>  #include 
> +#include 
>  #include 
> 
>  #include "private.h"
> --- a/xen/arch/x86/pv/emul-gate-op.c
> +++ b/xen/arch/x86/pv/emul-gate-op.c
> @@ -19,6 +19,7 @@
>   * along with this program; If not, see .
>   */
> 
> +#include 
>  #include 
>  #include 
>  #include 
> --- a/xen/include/asm-x86/hvm/svm/vmcb.h
> +++ b/xen/include/asm-x86/hvm/svm/vmcb.h
> @@ -20,8 +20,6 @@
>  #define __ASM_X86_HVM_SVM_VMCB_H__
> 
>  #include 
> -#include 
> -
> 
>  /* general 1 intercepts */
>  enum GenericIntercept1bits
> --- a/xen/include/asm-x86/hvm/vmx/vmx.h
> +++ b/xen/include/asm-x86/hvm/vmx/vmx.h
> @@ -97,6 +97,7 @@ void vmx_asm_do_vmentry(void);
>  void vmx_intr_assist(void);
>  void noreturn vmx_do_resume(struct vcpu *);
>  void vmx_vlapic_msr_changed(struct vcpu *v);
> +struct hvm_emulate_ctxt;
>  void vmx_realmode_emulate_one(struct hvm_emulate_ctxt *hvmemul_ctxt);
>  void vmx_realmode(struct cpu_user_regs *regs);
>  void vmx_update_debug_state(struct vcpu *v);
> 
> 
> ___
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 1/2] nvmx: fix handling of interrupts

[Roger Pau Monné]

On Mon, Feb 03, 2020 at 07:24:04AM +, Tian, Kevin wrote:
> > From: Roger Pau Monné 
> > Sent: Thursday, January 23, 2020 8:32 PM
> > 
> > On Tue, Jan 21, 2020 at 03:34:13AM +, Tian, Kevin wrote:
> > > > From: Roger Pau Monné 
> > > > Sent: Monday, January 20, 2020 6:19 PM
> > > >
> > > > On Sun, Jan 19, 2020 at 04:15:04AM +, Tian, Kevin wrote:
> > > > > > From: Roger Pau Monne 
> > > > > > Sent: Wednesday, January 8, 2020 6:39 PM
> > > > > >
> > > > > > When doing a virtual vmexit (ie: a vmexit handled by the L1 VMM)
> > > > > > interrupts shouldn't be injected using the virtual interrupt 
> > > > > > delivery
> > > > > > mechanism, and instead should be signaled in the vmcs using the exit
> > > > > > reason and the interruption-information field if the "Acknowledge
> > > > > > interrupt on exit" vmexit control is set.
> > > > > >
> > > > > > Remove the nvmx_update_apicv helper: it's bogus to attempt to inject
> > > > > > interrupts on virtual vmexit using the virtual interrupt delivery
> > > > > > assistance, and it's also bogus to ack interrupts without checking 
> > > > > > if
> > > > > > the vmexit "Acknowledge interrupt on exit" vmexit control is set.
> > > > > > nvmx_intr_intercept already handles interrupts correctly on virtual
> > > > > > vmexit.
> > > > > >
> > > > > > Note that this fixes the usage of x2APIC by the L1 VMM, at least 
> > > > > > when
> > > > > > the L1 VMM is Xen.
> > > > >
> > > > > while this fix makes sense to me, can you also test other L1 VMMs,
> > > > > so we don't overlook some other intentions covered or hidden by
> > > > > removed logic?
> > > >
> > > > I could test other hypervisors, but do we really expect anything
> > > > that's not Xen on Xen to work?
> > > >
> > > > I'm asking because that's the only combination that's actually tested
> > > > by osstest.
> > > >
> > > > Thanks, Roger.
> > >
> > > If others are OK with your assumption, then it's fine. I didn't tightly
> > > follow the nested virtualization requirements in Xen.
> > >
> > > On the other hand, I think this patch needs a revision. It is not bogus
> > > to use virtual interrupt delivery on virtual VMexit, if "Ack interrupt
> > > on exit" is off. In such case, the delivery doesn't happen until L1
> > > hypervisor enables interrupt to clear interrupt window. Then it does
> > > save one exit. The only bogus point is that nvmx_udpate_apicv doesn't
> > > check "Ack interrupt on exit". So I prefer to add such check there
> > > instead of completely removing this optimization.
> > 
> > I went back over this, and I'm still not sure calling
> > nvmx_update_apicv is actually required: AFAICT vmx_intr_assist will
> > already inject the interrupt correctly using virtual interrupt
> > delivery if left pending on the vlapic. I guess the code in
> > nvmx_update_apicv doesn't hurt as long as it's gated on "Ack on exit"
> > not being enabled, but it's likely redundant.
> 
> It's not redundant. If you look at the code sequence, vmx_intr_assist
> is invoked before nvmx_switch_guest. At that time, the L1 vCPU is still
> in nested guest mode, thereby nvmx_intr_intercept takes effect which
> injects the pending vector into vmcs02 and bypasses the remaining
> virtual interrupt delivery logic for vmcs01. That is the main reason, imo,
> why nvmx_update_apicv is introduced.
> 
> iiuc, nvmx_intr_intercept and nvmx_update_apicv work together to
> complete nested interrupt injection:
> 
> (1) If "Ack interrupt on exit" is on, the pending vector is acked by 
> the former and delivered in vvmexit information field.
> (2) If "Ack interrupt on exit" is off and no virtual interrupt delivery, 
> no ack and interrupt window is enabled by the former.
> (3) Otherwise, the vector is acked by the latter and delivered through
> virtual interrupt delivery (where vmcs01 has been switched in). 
> 
> However, there are two issues in current code. One is about (3), i.e.,
> as you identified nvmx_update_apicv shouldn't blindly enable the
> optimization without checking the Ack setting. the other is new 
> about (2) - currently nvmx_intr_interrupt always enables interrupt 
> window when the Ack setting is off, which actually negates the 
> optimization of nvmx_update_apicv. Both should be fixed.

OK, I think I got it. It's likely however that vmx_intr_assist is also
called with the vmcs already switched to vmcs01 (if there's a pending
softirq for example), I guess vmx_intr_assist also copes correctly
when called with the vmcs already switched?

Thanks, Roger.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] docs/xl: fix typo in xl.cfg

[Wei Liu]

On Mon, Feb 03, 2020 at 11:31:12AM +0100, Roger Pau Monne wrote:
> The name of the option is nographic.
> 
> Signed-off-by: Roger Pau Monné 

Acked-by: Wei Liu 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 2/9] xen: split parameter related definitions in own header file

[Jan Beulich]

On 21.01.2020 09:43, Juergen Gross wrote:
> Move the parameter related definitions from init.h into a new header
> file param.h. This will avoid include hell when new dependencies are
> added to parameter definitions.
> 
> Signed-off-by: Juergen Gross 

There was some re-basing necessary here, which I hope I didn't screw
up. I also decided to commit this despite, from a strictly formal
perspective, there still being missing acks here, based on this
being an entirely mechanical change to those files, and on the
grounds that further re-basing would likely turn out to be needed
with further delays here.

Jan

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 10/12] xen/build: use new $(c_flags) and $(a_flags) instead of $(CFLAGS)

[Anthony PERARD]

On Thu, Jan 30, 2020 at 02:29:42PM +0100, Jan Beulich wrote:
> On 17.01.2020 11:53, Anthony PERARD wrote:
> > We would like to calculate CFLAGS once and before calling Rules.mk,
> > so the variable CFLAGS needs to have the same value across the whole
> > build. Thus we need a new variable where some flags can change
> > depending on the target name.
> > 
> > Both the dependency and __OBJECT_FILE__ are such flags that change
> > depending on the target, so there are move out of $(CFLAGS).
> 
> I'm afraid I don't understand: Being a delayed expansion (or
> "recursively expanded") variable, what problem is there when CFLAGS
> references $@? Is there a plan to change the variable's flavor? If
> so, I'd like to ask for this to be mentioned here. "Calculate once",
> at least to me, doesn't imply this.

If I rewrite the first paragraph thus, would that be better?

In a later patch, we want to calculate the CFLAGS in xen/Makefile,
then export it. So have Rules.mk use a CFLAGS from the environment
variables. This will mean that if Rules.mk or a Makefile modify
CFLAGS for a target, the modification propagates to other targets.
Thus we will need a different variable name than the one from the
environment which can have a different value for each target.


> > @@ -141,9 +137,16 @@ endif
> >  # Always build obj-bin files as binary even if they come from C source. 
> >  $(obj-bin-y): CFLAGS := $(filter-out -flto,$(CFLAGS))
> >  
> > +c_flags = -MMD -MF $(@D)/.$(@F).d \
> > +  $(CFLAGS) \
> > +  '-D__OBJECT_FILE__="$@"'
> > +
> > +a_flags = -MMD -MF $(@D)/.$(@F).d \
> > +  $(AFLAGS)
> 
> Is there a reason both get extended over multiple lines?

Beside that it looks cleaner to me, not really.

> > --- a/xen/include/Makefile
> > +++ b/xen/include/Makefile
> > @@ -64,7 +64,7 @@ compat/%.h: compat/%.i Makefile 
> > $(BASEDIR)/tools/compat-build-header.py
> > mv -f $@.new $@
> >  
> >  compat/%.i: compat/%.c Makefile
> > -   $(CPP) $(filter-out -Wa$(comma)% -M% %.d -include 
> > %/include/xen/config.h,$(CFLAGS)) $(cppflags-y) -o $@ $<
> > +   $(CPP) $(filter-out -Wa$(comma)% -M% %.d -include 
> > %/include/xen/config.h,$(c_flags)) $(cppflags-y) -o $@ $<
> 
> I think this wants to continue to reference $(CFLAGS) and instead have
> the -M% and %.d patterns dropped. Similarly I guess as-insn in Config.mk
> could then have these two patterns dropped.

It's probably a good idea to keep using CFLAGS, I'll look into it.
As to change as-insn, I can move it out of Config.mk and then change it.
I'll look into that as well.

Thanks,

-- 
Anthony PERARD

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v2 1/4] x86/vvmx: fix virtual interrupt injection when Ack on exit control is used

[Roger Pau Monne]

When doing a virtual vmexit (ie: a vmexit handled by the L1 VMM)
interrupts shouldn't be injected using the virtual interrupt delivery
mechanism unless the Ack on exit vmexit control bit isn't set in the
nested vmcs.

Gate the call to nvmx_update_apicv helper on whether the nested vmcs
has the Ack on exit bit set in the vmexit control field.

Note that this fixes the usage of x2APIC by the L1 VMM, at least when
the L1 VMM is Xen.

Signed-off-by: Roger Pau Monné 
---
Changes since v1:
 - Call nvmx_update_apicv if the "Ack on exit" vmexit control bit
   isn't set.
---
 xen/arch/x86/hvm/vmx/vvmx.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index d8ab167d62..3d97a293b2 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1393,7 +1393,12 @@ static void virtual_vmexit(struct cpu_user_regs *regs)
 /* updating host cr0 to sync TS bit */
 __vmwrite(HOST_CR0, v->arch.hvm.vmx.host_cr0);
 
-if ( cpu_has_vmx_virtual_intr_delivery )
+if ( cpu_has_vmx_virtual_intr_delivery &&
+ /*
+  * Only inject the vector if the Ack on exit bit is not set, else the
+  * interrupt will be signaled in the vmcs VM_EXIT_INTR_INFO field.
+  */
+ !(get_vvmcs(v, VM_EXIT_CONTROLS) & VM_EXIT_ACK_INTR_ON_EXIT) )
 nvmx_update_apicv(v);
 
 nvcpu->nv_vmswitch_in_progress = 0;
-- 
2.25.0


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v2 3/4] x86/vvmx: don't enable interrupt window when using virt intr delivery

[Roger Pau Monne]

If virtual interrupt delivery is used to inject the interrupt to the
guest the interrupt window shouldn't be enabled, as the interrupt is
already injected using the GUEST_INTR_STATUS vmcs field.

Reported-by: Kevin Tian 
Signed-off-by: Roger Pau Monné 
---
Changes since v1:
 - New in this version.
---
 xen/arch/x86/hvm/vmx/intr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/hvm/vmx/intr.c b/xen/arch/x86/hvm/vmx/intr.c
index 2eaf3f8d36..49a1295f09 100644
--- a/xen/arch/x86/hvm/vmx/intr.c
+++ b/xen/arch/x86/hvm/vmx/intr.c
@@ -209,7 +209,7 @@ static int nvmx_intr_intercept(struct vcpu *v, struct 
hvm_intack intack)
 if ( unlikely(intack.source != hvm_intsrc_none) )
 vmx_enable_intr_window(v, intack);
 }
-else
+else if ( !cpu_has_vmx_virtual_intr_delivery )
 vmx_enable_intr_window(v, intack);
 
 return 1;
-- 
2.25.0


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v2 2/4] x86/vvmx: fix VM_EXIT_ACK_INTR_ON_EXIT handling

[Roger Pau Monne]

When VM_EXIT_ACK_INTR_ON_EXIT is set in the vmexit control vmcs
register the bit 31 of VM_EXIT_INTR_INFO must be 0, in order to denote
that the field doesn't contain any interrupt information. This is not
currently acknowledged as the field always get filled with valid
interrupt information, regardless of whether VM_EXIT_ACK_INTR_ON_EXIT
is set.

Fix this and only fill VM_EXIT_INTR_INFO when VM_EXIT_ACK_INTR_ON_EXIT
is not set. Note that this requires one minor change in
nvmx_update_apicv in order to obtain the interrupt information from
the internal state rather than the nested vmcs register.

Signed-off-by: Roger Pau Monné 
---
Changes since v1:
 - New in this version.
---
 xen/arch/x86/hvm/vmx/vvmx.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index 3d97a293b2..47eee1e5b9 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1283,6 +1283,7 @@ static void load_vvmcs_host_state(struct vcpu *v)
 static void sync_exception_state(struct vcpu *v)
 {
 struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
+uint32_t exit_ctrl = get_vvmcs(v, VM_EXIT_CONTROLS);
 
 if ( !(nvmx->intr.intr_info & INTR_INFO_VALID_MASK) )
 return;
@@ -1294,7 +1295,8 @@ static void sync_exception_state(struct vcpu *v)
 set_vvmcs(v, VM_EXIT_REASON, EXIT_REASON_EXTERNAL_INTERRUPT);
 set_vvmcs(v, EXIT_QUALIFICATION, 0);
 set_vvmcs(v, VM_EXIT_INTR_INFO,
-nvmx->intr.intr_info);
+  (exit_ctrl & VM_EXIT_ACK_INTR_ON_EXIT) ? nvmx->intr.intr_info
+ : 0);
 break;
 
 case X86_EVENTTYPE_HW_EXCEPTION:
@@ -1320,7 +1322,7 @@ static void nvmx_update_apicv(struct vcpu *v)
 {
 struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
 unsigned long reason = get_vvmcs(v, VM_EXIT_REASON);
-uint32_t intr_info = get_vvmcs(v, VM_EXIT_INTR_INFO);
+uint32_t intr_info = nvmx->intr.intr_info;
 
 if ( reason == EXIT_REASON_EXTERNAL_INTERRUPT &&
  nvmx->intr.source == hvm_intsrc_lapic &&
-- 
2.25.0


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v2 4/4] Revert "tools/libxc: disable x2APIC when using nested virtualization"

[Roger Pau Monne]

This reverts commit 7b3c5b70a32303b46d0d051e695f18d72cce5ed0 and
re-enables the usage of x2APIC with nested virtualization.

Signed-off-by: Roger Pau Monné 
Acked-by: Wei Liu 
---
 tools/libxc/xc_cpuid_x86.c | 11 ---
 1 file changed, 11 deletions(-)

diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
index ac38c1406e..2540aa1e1c 100644
--- a/tools/libxc/xc_cpuid_x86.c
+++ b/tools/libxc/xc_cpuid_x86.c
@@ -653,17 +653,6 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t 
domid,
 p->extd.itsc = true;
 p->basic.vmx = true;
 p->extd.svm = true;
-
-/*
- * BODGE: don't announce x2APIC mode when using nested virtualization,
- * as it doesn't work properly. This should be removed once the
- * underlying bug(s) are fixed.
- */
-rc = xc_hvm_param_get(xch, domid, HVM_PARAM_NESTEDHVM, &val);
-if ( rc )
-goto out;
-if ( val )
-p->basic.x2apic = false;
 }
 
 rc = x86_cpuid_copy_to_buffer(p, leaves, &nr_leaves);
-- 
2.25.0


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v2 0/4] x86/vvmx: fixes to interrupt injection

[Roger Pau Monne]

Hello,

The following series contain fixes for nested interrupt injection.

Thanks, Roger.

Roger Pau Monne (4):
  x86/vvmx: fix virtual interrupt injection when Ack on exit control is
used
  x86/vvmx: fix VM_EXIT_ACK_INTR_ON_EXIT handling
  x86/vvmx: don't enable interrupt window when using virt intr delivery
  Revert "tools/libxc: disable x2APIC when using nested virtualization"

 tools/libxc/xc_cpuid_x86.c  | 11 ---
 xen/arch/x86/hvm/vmx/intr.c |  2 +-
 xen/arch/x86/hvm/vmx/vvmx.c | 13 ++---
 3 files changed, 11 insertions(+), 15 deletions(-)

-- 
2.25.0


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 09/12] xen/build: include include/config/auto.conf in main Makefile

[Jan Beulich]

On 03.02.2020 12:45, Anthony PERARD wrote:
> On Thu, Jan 30, 2020 at 02:06:18PM +0100, Jan Beulich wrote:
>> On 17.01.2020 11:53, Anthony PERARD wrote:
>>> --- a/xen/Makefile
>>> +++ b/xen/Makefile
>>> @@ -49,7 +49,71 @@ default: build
>>>  .PHONY: dist
>>>  dist: install
>>>  
>>> -build install:: include/config/auto.conf
>>> +
>>> +ifndef root-make-done
>>> +# section to run before calling Rules.mk, but only once.
>>> +#
>>> +# To make sure we do not include .config for any of the *config targets
>>> +# catch them early, and hand them over to tools/kconfig/Makefile
>>> +
>>> +clean-targets := %clean
>>> +no-dot-config-targets := $(clean-targets) \
>>> +uninstall debug cloc \
>>> +cscope TAGS tags MAP gtags \
>>> +xenversion
>>> +
>>> +config-build   :=
>>
>> Is this actually needed? While correct (afaict) together with the
>> ifdef further down, I find this aspect of make behavior a little
>> confusing, and hence it would seem slightly better if there was
>> no empty definition of this symbol.
> 
> That's actually a very recent change in Linux source code. They used to
> use ifeq($(config-build),1) and ifeq($(config-build),0). I can certainly
> change back to use ifeq instead of ifdef.

Then perhaps, along the lines of ...

>>> +need-config:= 1
>>
>> Here and below, would it be possible to use y instead of 1, to
>> match how "true" gets expressed in various places elsewhere?
>> Or would there again be deviation-from-Linux concerns?
> 
> It's probably fine to use "y". I don't think it matter, we need to make
> quite a lot of changes compare to Linux anyway. I'll use "n" for the
> negative.

... this, also use y/n?

>>> +ifdef config-build
>>> +# 
>>> ===
>>> +# *config targets only - make sure prerequisites are updated, and descend
>>> +# in tools/kconfig to make the *config target
>>> +
>>> +config: FORCE
>>> +   $(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) 
>>> SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" $@
>>> +
>>> +# Config.mk tries to include .config file, don't try to remake it
>>> +%/.config: ;
>>
>> This didn't exist before - why is it needed all of the sudden?
> 
> It's because I'm introducing a new target "%config". So when make
> "-include $(XEN_ROOT)/.config" (as found in Config.mk) it check if the
> file needs to be rebuilt, and find %config and thus run kconfig to build
> .config.
> 
> Currently, Makefile list all the targets that needs to be built with
> kconfig.

Ah, I see - we didn't have a %config target anywhere at all.

Jan

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen-unstable: pci-passthrough regression bisected to: x86/smp: use APIC ALLBUT destination shorthand when possible

[Roger Pau Monné]

On Mon, Feb 03, 2020 at 09:33:51AM +0100, Sander Eikelenboom wrote:
> Hi Roger,
> 
> Last week I encountered an issue with the PCI-passthrough of a USB 
> controller. 
> In the guest I get:
> [ 1143.313756] xhci_hcd :00:05.0: xHCI host not responding to stop 
> endpoint command.
> [ 1143.334825] xhci_hcd :00:05.0: xHCI host controller not 
> responding, assume dead
> [ 1143.347364] xhci_hcd :00:05.0: HC died; cleaning up
> [ 1143.356407] usb 1-2: USB disconnect, device number 2
> 
> Bisection turned up as the culprit: 
>commit 5500d265a2a8fa63d60c08beb549de8ec82ff7a5
>x86/smp: use APIC ALLBUT destination shorthand when possible

Sorry to hear that, let see if we can figure out what's wrong.

> I verified by reverting that commit and now it works fine again.

Does the same controller work fine when used in dom0?

Thanks, Roger.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v6 05/11] x86/hyperv: setup hypercall page

[Wei Liu]

On Mon, Feb 03, 2020 at 11:41:57AM +, Durrant, Paul wrote:
> > -Original Message-
> > From: Jan Beulich 
> > Sent: 03 February 2020 11:39
> > To: Durrant, Paul 
> > Cc: Wei Liu ; Xen Development List  > de...@lists.xenproject.org>; Michael Kelley ; Wei
> > Liu ; Andrew Cooper ;
> > Roger Pau Monné 
> > Subject: Re: [PATCH v6 05/11] x86/hyperv: setup hypercall page
> > 
> > On 03.02.2020 12:37, Durrant, Paul wrote:
> > >> -Original Message-
> > >> From: Jan Beulich 
> > >> Sent: 03 February 2020 11:34
> > >> To: Durrant, Paul 
> > >> Cc: Wei Liu ; Xen Development List  > >> de...@lists.xenproject.org>; Michael Kelley ;
> > Wei
> > >> Liu ; Andrew Cooper ;
> > >> Roger Pau Monné 
> > >> Subject: Re: [PATCH v6 05/11] x86/hyperv: setup hypercall page
> > >>
> > >> On 03.02.2020 12:21, Durrant, Paul wrote:
> >  -Original Message-
> >  From: Jan Beulich 
> >  Sent: 03 February 2020 11:06
> >  To: Durrant, Paul 
> >  Cc: Wei Liu ; Xen Development List  >  de...@lists.xenproject.org>; Michael Kelley ;
> > >> Wei
> >  Liu ; Andrew Cooper ;
> >  Roger Pau Monné 
> >  Subject: Re: [PATCH v6 05/11] x86/hyperv: setup hypercall page
> > 
> >  On 03.02.2020 11:39, Durrant, Paul wrote:
> > >> -Original Message-
> > >> From: Wei Liu 
> > >> Sent: 31 January 2020 17:57
> > >> To: Xen Development List 
> > >> Cc: Durrant, Paul ; Michael Kelley
> > >> ; Wei Liu ; Wei Liu
> > >> ; Jan Beulich ; Andrew Cooper
> > >> ; Roger Pau Monné 
> > >> Subject: Re: [PATCH v6 05/11] x86/hyperv: setup hypercall page
> > >>
> > >> (Note to self)
> > >>
> > >> On Fri, Jan 31, 2020 at 05:49:24PM +, Wei Liu wrote:
> > >> [...]
> > >>> +static uint64_t generate_guest_id(void)
> > >>> +{
> > >>> +union hv_guest_os_id id;
> > >>> +
> > >>
> > >>id.raw = 0;
> > >
> > > Or just use a C99 initializer to set things up. A bit neater IMO.
> > 
> >  If you mean this for ...
> > 
> > >>> +id.vendor = HV_XEN_VENDOR_ID;
> > >>> +id.major = xen_major_version();
> > >>> +id.minor = xen_minor_version();
> > 
> >  ... these three fields, then this won't work with rather old gcc
> >  we still permit to be used. Using { .raw = 0 } would work afaict.
> > 
> > >>>
> > >>> Not even { .vendor = HV_XEN_VENDOR_ID } ?
> > >>
> > >> No, because of it being part of an unnamed (struct) member of
> > >> the union.
> > >
> > > Ok... shame. Presumably an empty initializer - {} -  would be ok?
> > 
> > I think so, yes. I understand you'd like this better than
> > { .raw = 0 } ?
> > 
> 
> Yes. In general, using a c99 initializer to explicitly set something
> to 0 seems a bit redundant to me.

Alright. I have changed it to

  union hv_guest_os_id id = {};

in my local branch.

Wei.

> 
>   Paul
> 
> > Jan

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen-unstable: pci-passthrough regression bisected to: x86/smp: use APIC ALLBUT destination shorthand when possible

[Sander Eikelenboom]

On 03/02/2020 13:23, Roger Pau Monné wrote:
> On Mon, Feb 03, 2020 at 09:33:51AM +0100, Sander Eikelenboom wrote:
>> Hi Roger,
>>
>> Last week I encountered an issue with the PCI-passthrough of a USB 
>> controller. 
>> In the guest I get:
>> [ 1143.313756] xhci_hcd :00:05.0: xHCI host not responding to stop 
>> endpoint command.
>> [ 1143.334825] xhci_hcd :00:05.0: xHCI host controller not 
>> responding, assume dead
>> [ 1143.347364] xhci_hcd :00:05.0: HC died; cleaning up
>> [ 1143.356407] usb 1-2: USB disconnect, device number 2
>>
>> Bisection turned up as the culprit: 
>>commit 5500d265a2a8fa63d60c08beb549de8ec82ff7a5
>>x86/smp: use APIC ALLBUT destination shorthand when possible
> 
> Sorry to hear that, let see if we can figure out what's wrong.

No problem, that is why I test stuff :)

>> I verified by reverting that commit and now it works fine again.
> 
> Does the same controller work fine when used in dom0?

Will test that, but as all other pci devices in dom0 work fine,
I assume this controller would also work fine in dom0 (as it has also
worked fine for ages with PCI-passthrough to that guest and still works
fine when reverting the referenced commit).

I don't know if your change can somehow have a side effect
on latency around the processing of pci-passthrough ?
(since the driver concluding that a device is non-responsive, will
probably be at least somewhat latency sensitive).

--
Sander

> Thanks, Roger.
> 


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 10/12] xen/build: use new $(c_flags) and $(a_flags) instead of $(CFLAGS)

[Jan Beulich]

On 03.02.2020 13:17, Anthony PERARD wrote:
> On Thu, Jan 30, 2020 at 02:29:42PM +0100, Jan Beulich wrote:
>> On 17.01.2020 11:53, Anthony PERARD wrote:
>>> We would like to calculate CFLAGS once and before calling Rules.mk,
>>> so the variable CFLAGS needs to have the same value across the whole
>>> build. Thus we need a new variable where some flags can change
>>> depending on the target name.
>>>
>>> Both the dependency and __OBJECT_FILE__ are such flags that change
>>> depending on the target, so there are move out of $(CFLAGS).
>>
>> I'm afraid I don't understand: Being a delayed expansion (or
>> "recursively expanded") variable, what problem is there when CFLAGS
>> references $@? Is there a plan to change the variable's flavor? If
>> so, I'd like to ask for this to be mentioned here. "Calculate once",
>> at least to me, doesn't imply this.
> 
> If I rewrite the first paragraph thus, would that be better?
> 
> In a later patch, we want to calculate the CFLAGS in xen/Makefile,
> then export it. So have Rules.mk use a CFLAGS from the environment
> variables. This will mean that if Rules.mk or a Makefile modify
> CFLAGS for a target, the modification propagates to other targets.
> Thus we will need a different variable name than the one from the
> environment which can have a different value for each target.

I think this is better, yes, albeit I'm still a little unhappy
about the uses of "target" here: It makes it look to me as if
this was primarily about things like

abc.o: CFLAGS += ...

where, unless its rule invokes make, I don't think the adjustment
would propagate anywhere. Instead aiui what you refer to is solely
an effect from the subdir traversal the build system does?

>>> @@ -141,9 +137,16 @@ endif
>>>  # Always build obj-bin files as binary even if they come from C source. 
>>>  $(obj-bin-y): CFLAGS := $(filter-out -flto,$(CFLAGS))
>>>  
>>> +c_flags = -MMD -MF $(@D)/.$(@F).d \
>>> +  $(CFLAGS) \
>>> +  '-D__OBJECT_FILE__="$@"'
>>> +
>>> +a_flags = -MMD -MF $(@D)/.$(@F).d \
>>> +  $(AFLAGS)
>>
>> Is there a reason both get extended over multiple lines?
> 
> Beside that it looks cleaner to me, not really.

Apparently a matter of taste then. I generally consider trailing
backslashes a little "unclean", and hence would prefer to avoid
them if I can.

>>> --- a/xen/include/Makefile
>>> +++ b/xen/include/Makefile
>>> @@ -64,7 +64,7 @@ compat/%.h: compat/%.i Makefile 
>>> $(BASEDIR)/tools/compat-build-header.py
>>> mv -f $@.new $@
>>>  
>>>  compat/%.i: compat/%.c Makefile
>>> -   $(CPP) $(filter-out -Wa$(comma)% -M% %.d -include 
>>> %/include/xen/config.h,$(CFLAGS)) $(cppflags-y) -o $@ $<
>>> +   $(CPP) $(filter-out -Wa$(comma)% -M% %.d -include 
>>> %/include/xen/config.h,$(c_flags)) $(cppflags-y) -o $@ $<
>>
>> I think this wants to continue to reference $(CFLAGS) and instead have
>> the -M% and %.d patterns dropped. Similarly I guess as-insn in Config.mk
>> could then have these two patterns dropped.
> 
> It's probably a good idea to keep using CFLAGS, I'll look into it.
> As to change as-insn, I can move it out of Config.mk and then change it.
> I'll look into that as well.

Thank you.

Jan

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen-unstable: pci-passthrough regression bisected to: x86/smp: use APIC ALLBUT destination shorthand when possible

[Roger Pau Monné]

On Mon, Feb 03, 2020 at 01:30:55PM +0100, Sander Eikelenboom wrote:
> On 03/02/2020 13:23, Roger Pau Monné wrote:
> > On Mon, Feb 03, 2020 at 09:33:51AM +0100, Sander Eikelenboom wrote:
> >> Hi Roger,
> >>
> >> Last week I encountered an issue with the PCI-passthrough of a USB 
> >> controller. 
> >> In the guest I get:
> >> [ 1143.313756] xhci_hcd :00:05.0: xHCI host not responding to stop 
> >> endpoint command.
> >> [ 1143.334825] xhci_hcd :00:05.0: xHCI host controller not 
> >> responding, assume dead
> >> [ 1143.347364] xhci_hcd :00:05.0: HC died; cleaning up
> >> [ 1143.356407] usb 1-2: USB disconnect, device number 2
> >>
> >> Bisection turned up as the culprit: 
> >>commit 5500d265a2a8fa63d60c08beb549de8ec82ff7a5
> >>x86/smp: use APIC ALLBUT destination shorthand when possible
> > 
> > Sorry to hear that, let see if we can figure out what's wrong.
> 
> No problem, that is why I test stuff :)
> 
> >> I verified by reverting that commit and now it works fine again.
> > 
> > Does the same controller work fine when used in dom0?
> 
> Will test that, but as all other pci devices in dom0 work fine,
> I assume this controller would also work fine in dom0 (as it has also
> worked fine for ages with PCI-passthrough to that guest and still works
> fine when reverting the referenced commit).

Is this the only device that fails to work when doing pci-passthrough,
or other devices also don't work with the mentioned change applied?

Have you tested on other boxes?

> I don't know if your change can somehow have a side effect
> on latency around the processing of pci-passthrough ?

Hm, the mentioned commit should speed up broadcast IPIs, but I don't
see how it could slow down other interrupts. Also I would think the
domain is not receiving interrupts from the device, rather than
interrupts being slow.

Can you also paste the output of lspci -v for that xHCI device from
dom0?

Thanks, Roger.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen-unstable: pci-passthrough regression bisected to: x86/smp: use APIC ALLBUT destination shorthand when possible

[Sander Eikelenboom]

On 03/02/2020 13:41, Roger Pau Monné wrote:
> On Mon, Feb 03, 2020 at 01:30:55PM +0100, Sander Eikelenboom wrote:
>> On 03/02/2020 13:23, Roger Pau Monné wrote:
>>> On Mon, Feb 03, 2020 at 09:33:51AM +0100, Sander Eikelenboom wrote:
 Hi Roger,

 Last week I encountered an issue with the PCI-passthrough of a USB 
 controller. 
 In the guest I get:
 [ 1143.313756] xhci_hcd :00:05.0: xHCI host not responding to stop 
 endpoint command.
 [ 1143.334825] xhci_hcd :00:05.0: xHCI host controller not 
 responding, assume dead
 [ 1143.347364] xhci_hcd :00:05.0: HC died; cleaning up
 [ 1143.356407] usb 1-2: USB disconnect, device number 2

 Bisection turned up as the culprit: 
commit 5500d265a2a8fa63d60c08beb549de8ec82ff7a5
x86/smp: use APIC ALLBUT destination shorthand when possible
>>>
>>> Sorry to hear that, let see if we can figure out what's wrong.
>>
>> No problem, that is why I test stuff :)
>>
 I verified by reverting that commit and now it works fine again.
>>>
>>> Does the same controller work fine when used in dom0?
>>
>> Will test that, but as all other pci devices in dom0 work fine,
>> I assume this controller would also work fine in dom0 (as it has also
>> worked fine for ages with PCI-passthrough to that guest and still works
>> fine when reverting the referenced commit).
> 
> Is this the only device that fails to work when doing pci-passthrough,
> or other devices also don't work with the mentioned change applied?
> 
> Have you tested on other boxes?
> 
>> I don't know if your change can somehow have a side effect
>> on latency around the processing of pci-passthrough ?
> 
> Hm, the mentioned commit should speed up broadcast IPIs, but I don't
> see how it could slow down other interrupts. Also I would think the
> domain is not receiving interrupts from the device, rather than
> interrupts being slow.
> 
> Can you also paste the output of lspci -v for that xHCI device from
> dom0?
> 
> Thanks, Roger.

Will do this evening including the testing in dom0 etc.
Will also see if there is any pattern when observing /proc/interrupts in
the guest.

Thanks,

Sander


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen-unstable: pci-passthrough regression bisected to: x86/smp: use APIC ALLBUT destination shorthand when possible

[Jan Beulich]

On 03.02.2020 13:41, Roger Pau Monné wrote:
> On Mon, Feb 03, 2020 at 01:30:55PM +0100, Sander Eikelenboom wrote:
>> On 03/02/2020 13:23, Roger Pau Monné wrote:
>>> On Mon, Feb 03, 2020 at 09:33:51AM +0100, Sander Eikelenboom wrote:
 Hi Roger,

 Last week I encountered an issue with the PCI-passthrough of a USB 
 controller. 
 In the guest I get:
 [ 1143.313756] xhci_hcd :00:05.0: xHCI host not responding to stop 
 endpoint command.
 [ 1143.334825] xhci_hcd :00:05.0: xHCI host controller not 
 responding, assume dead
 [ 1143.347364] xhci_hcd :00:05.0: HC died; cleaning up
 [ 1143.356407] usb 1-2: USB disconnect, device number 2

 Bisection turned up as the culprit: 
commit 5500d265a2a8fa63d60c08beb549de8ec82ff7a5
x86/smp: use APIC ALLBUT destination shorthand when possible
>>>
>>> Sorry to hear that, let see if we can figure out what's wrong.
>>
>> No problem, that is why I test stuff :)
>>
 I verified by reverting that commit and now it works fine again.
>>>
>>> Does the same controller work fine when used in dom0?
>>
>> Will test that, but as all other pci devices in dom0 work fine,
>> I assume this controller would also work fine in dom0 (as it has also
>> worked fine for ages with PCI-passthrough to that guest and still works
>> fine when reverting the referenced commit).
> 
> Is this the only device that fails to work when doing pci-passthrough,
> or other devices also don't work with the mentioned change applied?
> 
> Have you tested on other boxes?
> 
>> I don't know if your change can somehow have a side effect
>> on latency around the processing of pci-passthrough ?
> 
> Hm, the mentioned commit should speed up broadcast IPIs, but I don't
> see how it could slow down other interrupts. Also I would think the
> domain is not receiving interrupts from the device, rather than
> interrupts being slow.
> 
> Can you also paste the output of lspci -v for that xHCI device from
> dom0?

If this is AMD hardware, then another thing to try just to get an
additional data point would be limiting of CPUs used ("maxcpus="),
as that ought to suppress the actual sending of ALLBUT IPIs.

Jan

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v6 01/11] x86/hypervisor: make hypervisor_ap_setup return an error code

[Jan Beulich]

On 31.01.2020 18:49, Wei Liu wrote:
> We want to be able to handle AP setup error in the upper layer.
> 
> Signed-off-by: Wei Liu 
> ---
> v6:
> 1. Change map_vcpuinfo as well

And by implication then ...

> @@ -257,11 +257,17 @@ static void __init setup(void)
>  init_evtchn();
>  }
>  
> -static void ap_setup(void)
> +static int ap_setup(void)
>  {
> +int rc;
> +
>  set_vcpu_id();
> -map_vcpuinfo();
> -init_evtchn();
> +rc = map_vcpuinfo();
> +
> +if ( !rc )
> +init_evtchn();

... init_evtchn() as well?

Jan

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v6 03/11] x86: provide executable fixmap facility

[Jan Beulich]

On 31.01.2020 18:49, Wei Liu wrote:
> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -2,6 +2,8 @@
>  /* Modified for i386/x86-64 Xen by Keir Fraser */
>  
>  #include 
> +
> +#include 

I don't think you need this anymore? If so, with this dropped
Reviewed-by: Jan Beulich 
Otherwise please clarify.

Jan

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v6 04/11] x86/hypervisor: provide hypervisor_fixup_e820

[Jan Beulich]

On 31.01.2020 18:49, Wei Liu wrote:
> And implement the hook for Xen guest.
> 
> Signed-off-by: Wei Liu 

Reviewed-by: Jan Beulich 



___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen-unstable: pci-passthrough regression bisected to: x86/smp: use APIC ALLBUT destination shorthand when possible

[Roger Pau Monné]

On Mon, Feb 03, 2020 at 01:44:06PM +0100, Sander Eikelenboom wrote:
> On 03/02/2020 13:41, Roger Pau Monné wrote:
> > On Mon, Feb 03, 2020 at 01:30:55PM +0100, Sander Eikelenboom wrote:
> >> On 03/02/2020 13:23, Roger Pau Monné wrote:
> >>> On Mon, Feb 03, 2020 at 09:33:51AM +0100, Sander Eikelenboom wrote:
>  Hi Roger,
> 
>  Last week I encountered an issue with the PCI-passthrough of a USB 
>  controller. 
>  In the guest I get:
>  [ 1143.313756] xhci_hcd :00:05.0: xHCI host not responding to 
>  stop endpoint command.
>  [ 1143.334825] xhci_hcd :00:05.0: xHCI host controller not 
>  responding, assume dead
>  [ 1143.347364] xhci_hcd :00:05.0: HC died; cleaning up
>  [ 1143.356407] usb 1-2: USB disconnect, device number 2
> 
>  Bisection turned up as the culprit: 
> commit 5500d265a2a8fa63d60c08beb549de8ec82ff7a5
> x86/smp: use APIC ALLBUT destination shorthand when possible
> >>>
> >>> Sorry to hear that, let see if we can figure out what's wrong.
> >>
> >> No problem, that is why I test stuff :)
> >>
>  I verified by reverting that commit and now it works fine again.
> >>>
> >>> Does the same controller work fine when used in dom0?
> >>
> >> Will test that, but as all other pci devices in dom0 work fine,
> >> I assume this controller would also work fine in dom0 (as it has also
> >> worked fine for ages with PCI-passthrough to that guest and still works
> >> fine when reverting the referenced commit).
> > 
> > Is this the only device that fails to work when doing pci-passthrough,
> > or other devices also don't work with the mentioned change applied?
> > 
> > Have you tested on other boxes?
> > 
> >> I don't know if your change can somehow have a side effect
> >> on latency around the processing of pci-passthrough ?
> > 
> > Hm, the mentioned commit should speed up broadcast IPIs, but I don't
> > see how it could slow down other interrupts. Also I would think the
> > domain is not receiving interrupts from the device, rather than
> > interrupts being slow.
> > 
> > Can you also paste the output of lspci -v for that xHCI device from
> > dom0?
> > 
> > Thanks, Roger.
> 
> Will do this evening including the testing in dom0 etc.
> Will also see if there is any pattern when observing /proc/interrupts in
> the guest.

Thanks! I also have some trivial patch that I would like you to try,
just to discard send_IPI_mask clearing the scratch_cpumask under
another function feet.

Roger.
---
diff --git a/xen/arch/x86/smp.c b/xen/arch/x86/smp.c
index 65eb7cbda8..aeeb506155 100644
--- a/xen/arch/x86/smp.c
+++ b/xen/arch/x86/smp.c
@@ -66,7 +66,8 @@ static void send_IPI_shortcut(unsigned int shortcut, int 
vector,
 void send_IPI_mask(const cpumask_t *mask, int vector)
 {
 bool cpus_locked = false;
-cpumask_t *scratch = this_cpu(scratch_cpumask);
+static DEFINE_PER_CPU(cpumask_t, send_ipi_cpumask);
+cpumask_t *scratch = &this_cpu(send_ipi_cpumask);
 
 /*
  * This can only be safely used when no CPU hotplug or unplug operations


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v6 05/11] x86/hyperv: setup hypercall page

[Jan Beulich]

On 31.01.2020 18:49, Wei Liu wrote:
> +static void __init setup_hypercall_page(void)
> +{
> +union hv_x64_msr_hypercall_contents hypercall_msr;
> +union hv_guest_os_id guest_id;
> +unsigned long mfn;
> +
> +BUILD_BUG_ON(HV_HYP_PAGE_SHIFT != PAGE_SHIFT);
> +
> +rdmsrl(HV_X64_MSR_GUEST_OS_ID, guest_id.raw);
> +if ( !guest_id.raw )
> +{
> +guest_id.raw = generate_guest_id();
> +wrmsrl(HV_X64_MSR_GUEST_OS_ID, guest_id.raw);
> +}
> +
> +rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> +if ( !hypercall_msr.enable )
> +{
> +mfn = HV_HCALL_MFN;
> +hypercall_msr.enable = 1;
> +hypercall_msr.guest_physical_address = mfn;
> +wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> +} else {
> +mfn = hypercall_msr.guest_physical_address;
> +}

Nit: Style. Preferably omit the braces from "else" altogether.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v6 03/11] x86: provide executable fixmap facility

[Wei Liu]

On Mon, Feb 03, 2020 at 02:10:07PM +0100, Jan Beulich wrote:
> On 31.01.2020 18:49, Wei Liu wrote:
> > --- a/xen/arch/x86/xen.lds.S
> > +++ b/xen/arch/x86/xen.lds.S
> > @@ -2,6 +2,8 @@
> >  /* Modified for i386/x86-64 Xen by Keir Fraser */
> >  
> >  #include 
> > +
> > +#include 
> 
> I don't think you need this anymore? If so, with this dropped

I think it can be dropped.

> Reviewed-by: Jan Beulich 

Thanks.

Wei.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v6 06/11] x86/hyperv: provide Hyper-V hypercall functions

[Jan Beulich]

On 31.01.2020 18:49, Wei Liu wrote:
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -375,9 +375,11 @@ void __init arch_init_memory(void)
>  }
>  #endif
>  
> -/* Generate a symbol to be used in linker script */
> +/* Generate symbols to be used in linker script */
>  asm ( ".equ FIXADDR_X_SIZE, %P0; .global FIXADDR_X_SIZE"
>:: "i" (FIXADDR_X_SIZE) );
> +asm ( ".equ HV_HCALL_PAGE, %P0; .global HV_HCALL_PAGE"
> +  :: "i" (__fix_x_to_virt(FIX_X_HYPERV_HCALL)) );

Would this even build without CONFIG_HYPERV_GUEST? In any event
it doesn't belong here, but should go in a Hyper-V specific
file.

Seeing you now need two of these, how about macro-izing the
construct?

Jan

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v6 06/11] x86/hyperv: provide Hyper-V hypercall functions

[Wei Liu]

On Mon, Feb 03, 2020 at 02:26:24PM +0100, Jan Beulich wrote:
> On 31.01.2020 18:49, Wei Liu wrote:
> > --- a/xen/arch/x86/mm.c
> > +++ b/xen/arch/x86/mm.c
> > @@ -375,9 +375,11 @@ void __init arch_init_memory(void)
> >  }
> >  #endif
> >  
> > -/* Generate a symbol to be used in linker script */
> > +/* Generate symbols to be used in linker script */
> >  asm ( ".equ FIXADDR_X_SIZE, %P0; .global FIXADDR_X_SIZE"
> >:: "i" (FIXADDR_X_SIZE) );
> > +asm ( ".equ HV_HCALL_PAGE, %P0; .global HV_HCALL_PAGE"
> > +  :: "i" (__fix_x_to_virt(FIX_X_HYPERV_HCALL)) );
> 
> Would this even build without CONFIG_HYPERV_GUEST? In any event
> it doesn't belong here, but should go in a Hyper-V specific
> file.
> 

Good catch. When I did my full build tests it was done with my previous
version.

I can move this to hyperv.c.

> Seeing you now need two of these, how about macro-izing the
> construct?

What name would you suggest? I'm thinking about GEN_XEN_LDS_SYMBOL.

And presumably I should put it in xen/lib.h?

Wei.

> 
> Jan

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [qemu-mainline test] 146696: regressions - FAIL

[osstest service owner]

flight 146696 qemu-mainline real [real]
http://logs.test-lab.xenproject.org/osstest/logs/146696/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-arm64   6 xen-buildfail REGR. vs. 144861
 build-arm64-xsm   6 xen-buildfail REGR. vs. 144861
 build-armhf   6 xen-buildfail REGR. vs. 144861
 build-amd64   6 xen-buildfail REGR. vs. 144861
 build-amd64-xsm   6 xen-buildfail REGR. vs. 144861
 build-i386-xsm6 xen-buildfail REGR. vs. 144861
 build-i3866 xen-buildfail REGR. vs. 144861

Tests which did not succeed, but are not blocking:
 build-arm64-libvirt   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-rtds  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-raw1 build-check(1)   blocked  n/a
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-i386-xl1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-shadow1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-amd64  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-seattle   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  1 build-check(1)blocked n/a
 test-amd64-amd64-xl-pvhv2-intel  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ws16-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-win7-amd64  1 build-check(1)  blocked n/a
 test-armhf-armhf-xl-credit1   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-pvshim1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-credit2   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-vhd  1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-i386  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl   1 build-check(1)   blocked  n/a
 test-amd64-amd64-qemuu-nested-amd  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-ws16-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-debianhvm-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked 
n/a
 test-amd64-i386-pair  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm  1 build-check(1) blocked n/a
 test-amd64-i386-libvirt-xsm   1 build-check(1)   blocked  n/a
 build-armhf-libvirt   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-arndale   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-amd64-i386-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-shadow 1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-vhd   1 build-check(1)   blocked  n/a
 test-amd64-amd64-i386-pvgrub  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-credit1   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-thunderx  1 build-check(1)   blocked  n/a
 test-amd64-i386-qemuu-rhel6hvm-intel  1 build-check(1) blocked n/a
 test-amd64-amd64-xl-pvhv2-amd  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked 
n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-win7-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow  1 build-check(1) blocked n/a
 test-armhf-armhf-xl-credit2   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-xsm   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-xsm1 build-check(1)   blocked  n/a
 test-amd64-amd64-pair 1 build-check(1)   blocked  n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qcow2 1 build-

Re: [Xen-devel] [PATCH v6 05/11] x86/hyperv: setup hypercall page

[Wei Liu]

On Mon, Feb 03, 2020 at 02:21:14PM +0100, Jan Beulich wrote:
> On 31.01.2020 18:49, Wei Liu wrote:
> > +static void __init setup_hypercall_page(void)
> > +{
> > +union hv_x64_msr_hypercall_contents hypercall_msr;
> > +union hv_guest_os_id guest_id;
> > +unsigned long mfn;
> > +
> > +BUILD_BUG_ON(HV_HYP_PAGE_SHIFT != PAGE_SHIFT);
> > +
> > +rdmsrl(HV_X64_MSR_GUEST_OS_ID, guest_id.raw);
> > +if ( !guest_id.raw )
> > +{
> > +guest_id.raw = generate_guest_id();
> > +wrmsrl(HV_X64_MSR_GUEST_OS_ID, guest_id.raw);
> > +}
> > +
> > +rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> > +if ( !hypercall_msr.enable )
> > +{
> > +mfn = HV_HCALL_MFN;
> > +hypercall_msr.enable = 1;
> > +hypercall_msr.guest_physical_address = mfn;
> > +wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> > +} else {
> > +mfn = hypercall_msr.guest_physical_address;
> > +}
> 
> Nit: Style. Preferably omit the braces from "else" altogether.

Fixed.

Wei.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 1/9] xen: add a generic way to include binary files as variables

[Jan Beulich]

On 21.01.2020 09:43, Juergen Gross wrote:
> --- /dev/null
> +++ b/xen/tools/binfile
> @@ -0,0 +1,29 @@
> +#!/bin/sh
> +# usage: binfile [-i]   
> +# -i add to .init.rodata (default: .rodata) section
> +
> +[ "$1" = "-i" ] && {
> +shift
> +section=".init"
> +}
> +
> +target=$1
> +binsource=$2
> +varname=$3
> +
> +cat <$target
> +#include 
> +
> +.section $section.rodata, "a", %progbits
> +
> +.global $varname
> +$varname:
> +.incbin "$binsource"
> +.Lend:
> +
> +.type $varname, %object
> +.size $varname, . - $varname

I'd prefer if you used .Lend here as well.

I wonder whether, right from the beginning, there wouldn't better
be a way to also request better than byte alignment for such a
blob.

> --- a/xen/xsm/flask/Makefile
> +++ b/xen/xsm/flask/Makefile
> @@ -30,6 +30,9 @@ $(AV_H_FILES): $(AV_H_DEPEND)
>  obj-bin-$(CONFIG_XSM_FLASK_POLICY) += flask-policy.o
>  flask-policy.o: policy.bin
>  
> +flask-policy.S: $(XEN_ROOT)/xen/tools/binfile
> + $(XEN_ROOT)/xen/tools/binfile -i $@ policy.bin xsm_flask_init_policy

Doesn't objcopy provide a means to convert a binary blob into
an ELF object containing the binary data from the input file?
If so, why involve the assembler and an intermediate file here?

Jan

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v6 06/11] x86/hyperv: provide Hyper-V hypercall functions

[Jan Beulich]

On 03.02.2020 14:31, Wei Liu wrote:
> On Mon, Feb 03, 2020 at 02:26:24PM +0100, Jan Beulich wrote:
>> On 31.01.2020 18:49, Wei Liu wrote:
>>> --- a/xen/arch/x86/mm.c
>>> +++ b/xen/arch/x86/mm.c
>>> @@ -375,9 +375,11 @@ void __init arch_init_memory(void)
>>>  }
>>>  #endif
>>>  
>>> -/* Generate a symbol to be used in linker script */
>>> +/* Generate symbols to be used in linker script */
>>>  asm ( ".equ FIXADDR_X_SIZE, %P0; .global FIXADDR_X_SIZE"
>>>:: "i" (FIXADDR_X_SIZE) );
>>> +asm ( ".equ HV_HCALL_PAGE, %P0; .global HV_HCALL_PAGE"
>>> +  :: "i" (__fix_x_to_virt(FIX_X_HYPERV_HCALL)) );
>>
>> Would this even build without CONFIG_HYPERV_GUEST? In any event
>> it doesn't belong here, but should go in a Hyper-V specific
>> file.
>>
> 
> Good catch. When I did my full build tests it was done with my previous
> version.
> 
> I can move this to hyperv.c.
> 
>> Seeing you now need two of these, how about macro-izing the
>> construct?
> 
> What name would you suggest? I'm thinking about GEN_XEN_LDS_SYMBOL.

In principle this isn't limiting things to use by xen.lds, so
I'd prefer to not encode such in the name. asm_constant()? Or
all caps if so preferred by others?

> And presumably I should put it in xen/lib.h?

Would be nice to have it there, but I'm afraid this is against
what gcc documents. Hence if anything the P would need to be
abstracted away as a per-arch thing. If you don't want to go
this far, asm_defns.h might be the best fit among the x86
headers.

Jan

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v6 06/11] x86/hyperv: provide Hyper-V hypercall functions

[Wei Liu]

On Mon, Feb 03, 2020 at 02:48:42PM +0100, Jan Beulich wrote:
> On 03.02.2020 14:31, Wei Liu wrote:
> > On Mon, Feb 03, 2020 at 02:26:24PM +0100, Jan Beulich wrote:
> >> On 31.01.2020 18:49, Wei Liu wrote:
> >>> --- a/xen/arch/x86/mm.c
> >>> +++ b/xen/arch/x86/mm.c
> >>> @@ -375,9 +375,11 @@ void __init arch_init_memory(void)
> >>>  }
> >>>  #endif
> >>>  
> >>> -/* Generate a symbol to be used in linker script */
> >>> +/* Generate symbols to be used in linker script */
> >>>  asm ( ".equ FIXADDR_X_SIZE, %P0; .global FIXADDR_X_SIZE"
> >>>:: "i" (FIXADDR_X_SIZE) );
> >>> +asm ( ".equ HV_HCALL_PAGE, %P0; .global HV_HCALL_PAGE"
> >>> +  :: "i" (__fix_x_to_virt(FIX_X_HYPERV_HCALL)) );
> >>
> >> Would this even build without CONFIG_HYPERV_GUEST? In any event
> >> it doesn't belong here, but should go in a Hyper-V specific
> >> file.
> >>
> > 
> > Good catch. When I did my full build tests it was done with my previous
> > version.
> > 
> > I can move this to hyperv.c.
> > 
> >> Seeing you now need two of these, how about macro-izing the
> >> construct?
> > 
> > What name would you suggest? I'm thinking about GEN_XEN_LDS_SYMBOL.
> 
> In principle this isn't limiting things to use by xen.lds, so
> I'd prefer to not encode such in the name. asm_constant()? Or
> all caps if so preferred by others?

I am certainly okay with ASM_CONSTANT().

> 
> > And presumably I should put it in xen/lib.h?
> 
> Would be nice to have it there, but I'm afraid this is against
> what gcc documents. Hence if anything the P would need to be
> abstracted away as a per-arch thing. If you don't want to go
> this far, asm_defns.h might be the best fit among the x86
> headers.

OK. asm_defns.h it is. Arm doesn't need this for now anyway.

Wei.

> 
> Jan

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 12/12] xen/build: have the root Makefile generates the CFLAGS

[Anthony PERARD]

On Thu, Jan 30, 2020 at 03:33:15PM +0100, Jan Beulich wrote:
> On 17.01.2020 11:53, Anthony PERARD wrote:
> > Instead of generating the CFLAGS in Rules.mk everytime we enter a new
> > subdirectory, we are going to generate most of them a single time, and
> > export the result in the environment so that Rules.mk can use it.  The
> > only flags left to generates are the one that depends on the targets,
> > but the variable $(c_flags) takes care of that.
> > 
> > Arch specific CFLAGS are generated by a new file "arch/*/arch.mk"
> > which is included by the root Makefile.
> > 
> > In order to allow some variable that are specific to one arch and
> > needs to be regenerated for each target, there's a new variable
> > $(arch_ccflags).
> 
> And simply adding to c_flags is considered bad? Or does not work?

I could try to add directly to c_flags, it might not be that bad.
arch_ccflags is my invention, Linux doesn't have something similar.

> > --- a/xen/Makefile
> > +++ b/xen/Makefile
> > @@ -113,6 +113,76 @@ $(KCONFIG_CONFIG):
> >  %/auto.conf %/auto.conf.cmd: $(KCONFIG_CONFIG)
> > $(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) 
> > SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" syncconfig
> >  
> > +ifeq ($(CONFIG_DEBUG),y)
> > +CFLAGS += -O1
> > +else
> > +CFLAGS += -O2
> > +endif
> 
> Why does this start with +=, not := (or = )?

Makefile includes Config.mk before these lines, so having += simply add
to the CFLAGS already generated by Config.mk.

> > +ifeq ($(CONFIG_FRAME_POINTER),y)
> > +CFLAGS += -fno-omit-frame-pointer
> > +else
> > +CFLAGS += -fomit-frame-pointer
> > +endif
> > +
> > +CFLAGS += -nostdinc -fno-builtin -fno-common
> > +CFLAGS += -Werror -Wredundant-decls -Wno-pointer-arith
> > +$(call cc-option-add,CFLAGS,CC,-Wvla)
> > +CFLAGS += -pipe -D__XEN__ -include $(BASEDIR)/include/xen/config.h
> > +CFLAGS-$(CONFIG_DEBUG_INFO) += -g
> > +
> > +ifneq ($(CONFIG_CC_IS_CLANG),y)
> > +# Clang doesn't understand this command line argument, and doesn't appear 
> > to
> > +# have an suitable alternative.  The resulting compiled binary does 
> > function,
> > +# but has an excessively large symbol table.
> > +CFLAGS += -Wa,--strip-local-absolute
> > +endif
> > +
> > +AFLAGS-y+= -D__ASSEMBLY__
> 
> Why not just AFLAGS? I think in a overhaul like what you do,
> anomalies like this one would better be eliminated. The -y
> forms should be added into the base variables (like you do ...

I wanted to avoid too much modification, and mostly want to move the code
around. So it would be easier to check that the commit doesn't introduce
errors.  So, if your are fine with patch that move code and modify it,
I'll fix some of the oddities. (Or I can have another patch for it.)

> > +CFLAGS += $(CFLAGS-y)
> 
> ... here), but be added to only via CFLAGS-$(variable)
> constructs. Or otherwise there should be only CFLAGS-y, and
> no plain CFLAGS at all.
> 
> > +# allow extra CFLAGS externally via EXTRA_CFLAGS_XEN_CORE
> > +CFLAGS += $(EXTRA_CFLAGS_XEN_CORE)
> > +
> > +# Most CFLAGS are safe for assembly files:
> > +#  -std=gnu{89,99} gets confused by #-prefixed end-of-line comments
> > +#  -flto makes no sense and annoys clang
> > +AFLAGS += $(AFLAGS-y) $(filter-out -std=gnu% -flto,$(CFLAGS))
> > +
> > +# LDFLAGS are only passed directly to $(LD)
> > +LDFLAGS += $(LDFLAGS_DIRECT)
> > +
> > +LDFLAGS += $(LDFLAGS-y)
> 
> These two could be folded.
> 
> > +ifeq ($(CONFIG_COVERAGE),y)
> > +ifeq ($(CONFIG_CC_IS_CLANG),y)
> > +COV_FLAGS := -fprofile-instr-generate -fcoverage-mapping
> > +else
> > +COV_FLAGS := -fprofile-arcs -ftest-coverage
> > +endif
> > +else
> > +COV_FLAGS :=
> > +endif
> 
> COV_FLAGS gets propagated through the environment, despite being
> invariant. Can't this stay in Rules.mk?

I guess that's fine.

> > --- a/xen/arch/x86/Rules.mk
> > +++ b/xen/arch/x86/Rules.mk
> > @@ -1,89 +1,10 @@
> >  
> >  # x86-specific definitions
> >  
> > -XEN_IMG_OFFSET := 0x20
> > -
> > -CFLAGS += -I$(BASEDIR)/include
> > -CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-generic
> > -CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-default
> > -CFLAGS += -DXEN_IMG_OFFSET=$(XEN_IMG_OFFSET)
> > -CFLAGS += '-D__OBJECT_LABEL__=$(subst /,$$,$(subst -,_,$(subst 
> > $(BASEDIR)/,,$(CURDIR))/$@))'
> > -
> > -# Prevent floating-point variables from creeping into Xen.
> > -CFLAGS += -msoft-float
> > -
> > -ifeq ($(CONFIG_CC_IS_CLANG),y)
> > -# Note: Any test which adds -no-integrated-as will cause subsequent tests 
> > to
> > -# succeed, and not trigger further additions.
> > -#
> > -# The tests to select whether the integrated assembler is usable need to 
> > happen
> > -# before testing any assembler features, or else the result of the tests 
> > would
> > -# be stale if the integrated assembler is not used.
> > -
> > -# Older clang's built-in assembler doesn't understand .skip with labels:
> > -# https://bugs.llvm.org/show_bug.cgi?id=27369
> > -$(c

Re: [Xen-devel] [PATCH v6 01/11] x86/hypervisor: make hypervisor_ap_setup return an error code

[Wei Liu]

On Mon, Feb 03, 2020 at 01:56:48PM +0100, Jan Beulich wrote:
> On 31.01.2020 18:49, Wei Liu wrote:
> > We want to be able to handle AP setup error in the upper layer.
> > 
> > Signed-off-by: Wei Liu 
> > ---
> > v6:
> > 1. Change map_vcpuinfo as well
> 
> And by implication then ...
> 
> > @@ -257,11 +257,17 @@ static void __init setup(void)
> >  init_evtchn();
> >  }
> >  
> > -static void ap_setup(void)
> > +static int ap_setup(void)
> >  {
> > +int rc;
> > +
> >  set_vcpu_id();
> > -map_vcpuinfo();
> > -init_evtchn();
> > +rc = map_vcpuinfo();
> > +
> > +if ( !rc )
> > +init_evtchn();
> 
> ... init_evtchn() as well?

OK. I can change that too.

Wei.

> 
> Jan

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 5/8] xen/vmap: allow vmap() to be called during early boot

[Julien Grall]

Hi David,

On 01/02/2020 00:33, David Woodhouse wrote:

From: David Woodhouse 


I am a bit concerned with this change, particularly the consequence this 
have for the page-tables. There is an assumption that intermediate 
page-tables allocated via the boot allocator will never be freed.


On x86, a call to vunmap() will not free page-tables, but a subsequent 
call to vmap() may free it depending on the mapping size. So we would 
call free_domheap_pages() rather than init_heap_pages().


I am not entirely sure what is the full consequence, but I think this is 
a call for investigation and write it down a summary in the commit message.


Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 1/9] xen: add a generic way to include binary files as variables

[Jürgen Groß]

On 03.02.20 14:39, Jan Beulich wrote:

On 21.01.2020 09:43, Juergen Gross wrote:

--- /dev/null
+++ b/xen/tools/binfile
@@ -0,0 +1,29 @@
+#!/bin/sh
+# usage: binfile [-i]   
+# -i add to .init.rodata (default: .rodata) section
+
+[ "$1" = "-i" ] && {
+shift
+section=".init"
+}
+
+target=$1
+binsource=$2
+varname=$3
+
+cat <$target
+#include 
+
+.section $section.rodata, "a", %progbits
+
+.global $varname
+$varname:
+.incbin "$binsource"
+.Lend:
+
+.type $varname, %object
+.size $varname, . - $varname


I'd prefer if you used .Lend here as well.


Okay.


I wonder whether, right from the beginning, there wouldn't better
be a way to also request better than byte alignment for such a
blob.


I can add that. What about "-a " for 2^n alignment?




--- a/xen/xsm/flask/Makefile
+++ b/xen/xsm/flask/Makefile
@@ -30,6 +30,9 @@ $(AV_H_FILES): $(AV_H_DEPEND)
  obj-bin-$(CONFIG_XSM_FLASK_POLICY) += flask-policy.o
  flask-policy.o: policy.bin
  
+flask-policy.S: $(XEN_ROOT)/xen/tools/binfile

+   $(XEN_ROOT)/xen/tools/binfile -i $@ policy.bin xsm_flask_init_policy


Doesn't objcopy provide a means to convert a binary blob into
an ELF object containing the binary data from the input file?
If so, why involve the assembler and an intermediate file here?


I can see how to add a symbol for that purpose using a dedicated section
for each blob, but how to add the size information of the blob?

In the end I just followed commit 8d5671eb31e4bf for the inclusion of
the blob.


Juergen

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 6/8] x86/setup: move vm_init() before end_boot_allocator()

[David Woodhouse]

On Mon, 2020-02-03 at 11:10 +, Xia, Hongyan wrote:
> Is there any problem to move vm_init() even earlier than this, like
> right after init_frametable()? ACPI and NUMA functions need a couple of
> things permanently mapped. 

You want it sooner than that, don't you? The code calls
acpi_boot_table_init() and srat_parse_regions() while looping over the
e820 regions, before init_frametable(). But that's OK; you can call
vm_init() the moment you have the pages in the boot allocator to
support it.

So you can do something like the hack below, for example.

This boots in all three of the liveupdate=, <4GiB, >4GiB cases on
x86_64 — but will probably break Arm unless you make vm_init() run soon
enough there too, and will potentially run vm_init() more than once on
x86_64 if acpi_boot_table_init() fails the first time(s).

But as a proof-of-concept, sure.

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 2071e5acee..8aee55f31a 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1487,12 +1487,16 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 continue;
 
 if ( !acpi_boot_table_init_done &&
- s >= (1ULL << 32) &&
- !acpi_boot_table_init() )
+ s >= (1ULL << 32) )
 {
-acpi_boot_table_init_done = true;
-srat_parse_regions(s);
-setup_max_pdx(raw_max_page);
+printk("acpi/vm init\n");
+vm_init(); // XX: not idempotent
+if ( !acpi_boot_table_init() )
+{
+acpi_boot_table_init_done = true;
+srat_parse_regions(s);
+setup_max_pdx(raw_max_page);
+}
 }
 
 if ( pfn_to_pdx((e - 1) >> PAGE_SHIFT) >= max_pdx )
@@ -1677,14 +1681,16 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 init_frametable();
 
 if ( !acpi_boot_table_init_done )
+{
+printk("Late vm/acpi init\n");
+vm_init();
 acpi_boot_table_init();
+}
 
 acpi_numa_init();
 
 numa_initmem_init(0, raw_max_page);
 
-vm_init();
-
 if ( lu_breadcrumb_phys )
 {
 lu_stream_map(&lu_stream, lu_mfnlist_phys, lu_nr_pages);
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index f1e7d81edc..e5d938f8ca 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -322,7 +322,7 @@ void __init init_boot_pages(paddr_t ps, paddr_t pe)
  MAX_ORDER + 1);
 #endif
 BUILD_BUG_ON(sizeof(frame_table->u) != sizeof(unsigned long));
-
+printk("init boot pages %lx %lx\n", ps, pe);
 ps = round_pgup(ps);
 pe = round_pgdown(pe);
 if ( pe <= ps )
@@ -395,7 +395,7 @@ mfn_t __init alloc_boot_pages(unsigned long nr_pfns, 
unsigned long pfn_align)
 unsigned int i = nr_bootmem_regions;
 
 BUG_ON(!nr_bootmem_regions);
-
+printk("alloc_boot_pages %ld\n", nr_pfns);
 while ( i-- )
 {
 struct bootmem_region *r = &bootmem_region_list[i];
diff --git a/xen/drivers/acpi/osl.c b/xen/drivers/acpi/osl.c
index 4c8bb7839e..b7fcee408e 100644
--- a/xen/drivers/acpi/osl.c
+++ b/xen/drivers/acpi/osl.c
@@ -92,10 +92,11 @@ acpi_physical_address __init acpi_os_get_root_pointer(void)
 void __iomem *
 acpi_os_map_memory(acpi_physical_address phys, acpi_size size)
 {
-   if (system_state >= SYS_STATE_boot) {
+   if (1 || system_state >= SYS_STATE_boot) {
mfn_t mfn = _mfn(PFN_DOWN(phys));
unsigned int offs = phys & (PAGE_SIZE - 1);
 
+printk("ACPI vmap %08lx\n", phys);
/* The low first Mb is always mapped on x86. */
if (IS_ENABLED(CONFIG_X86) && !((phys + size - 1) >> 20))
return __va(phys);
@@ -114,7 +115,7 @@ void acpi_os_unmap_memory(void __iomem * virt, acpi_size 
size)
return;
}
 
-   if (system_state >= SYS_STATE_boot)
+   if (1 || system_state >= SYS_STATE_boot)
vunmap((void *)((unsigned long)virt & PAGE_MASK));
 }
 



smime.p7s
Description: S/MIME cryptographic signature
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [ovmf test] 146694: regressions - FAIL

[osstest service owner]

flight 146694 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/146694/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 145767
 test-amd64-amd64-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 
145767

version targeted for testing:
 ovmf eafd990f2606431d45cf0bbdbfee6d5959628de7
baseline version:
 ovmf 70911f1f4aee0366b6122f2b90d367ec0f066beb

Last test of basis   145767  2020-01-08 00:39:09 Z   26 days
Failing since145774  2020-01-08 02:50:20 Z   26 days   98 attempts
Testing same since   146634  2020-02-01 01:54:23 Z2 days9 attempts


People who touched revisions under test:
  Aaron Li 
  Albecki, Mateusz 
  Anthony PERARD 
  Ard Biesheuvel 
  Ashish Singhal 
  Bob Feng 
  Brian R Haug 
  Eric Dong 
  Fan, ZhijuX 
  Hao A Wu 
  Jason Voelz 
  Jian J Wang 
  Krzysztof Koch 
  Laszlo Ersek 
  Leif Lindholm 
  Li, Aaron 
  Liming Gao 
  Mateusz Albecki 
  Michael D Kinney 
  Michael Kubacki 
  Pavana.K 
  Philippe Mathieu-Daud? 
  Philippe Mathieu-Daude 
  Siyuan Fu 
  Siyuan, Fu 
  Sudipto Paul 
  Vitaly Cheptsov 
  Vitaly Cheptsov via Groups.Io 
  Wei6 Xu 
  Xu, Wei6 
  Zhiguang Liu 
  Zhiju.Fan 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 fail
 test-amd64-i386-xl-qemuu-ovmf-amd64  fail



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Not pushing.

(No revision log; it would be 1780 lines long.)

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 11/12] xen/build: introduce ccflags-y and CFLAGS_$@

[Anthony PERARD]

On Thu, Jan 30, 2020 at 02:39:43PM +0100, Jan Beulich wrote:
> On 17.01.2020 11:53, Anthony PERARD wrote:
> I have to admit that I'm also a little puzzled by the naming, no
> matter that it's taken from Linux. It doesn't really seem to fit
> CFLAGS/AFLAGS and c_flags/a_flags.

So I've look into the history of Linux, ccflags-y was introduce to get
rid of EXTRA_CFLAGS and especially to have the -y part in the name of
the variable.

So, instead of ccflags-y and the like, we could use CFLAGS-y in Makefile
of subdirectories.

For makefiles in subdir, Linux has:
CFLAGS_$@
CFLAGS_REMOVE_$@
ccflags-y
subdir-ccflags-y
so CFLAGS-y would be better (and we can think about the subdir one
later).

-- 
Anthony PERARD

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v6 11/11] x86/hyperv: setup VP assist page

[Roger Pau Monné]

On Fri, Jan 31, 2020 at 05:49:30PM +, Wei Liu wrote:
> VP assist page is rather important as we need to toggle some bits in it
> for efficient nested virtualisation.
> 
> Signed-off-by: Wei Liu 

Reviewed-by: Roger Pau Monné 

Thanks, Roger.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2.1 13/12] Makefile: Fix install-tests

[Anthony PERARD]

On Thu, Jan 30, 2020 at 12:37:17PM +0100, Jan Beulich wrote:
> On 21.01.2020 14:59, Anthony PERARD wrote:
> > The top-level makefile make uses of internal implementation detail of
> > the xen build system. Avoid that by creating a new target
> > "install-tests" in xen/Makefile, and by fixing the top-level Makefile
> > to not call xen/Rules.mk anymore.
> > 
> > Signed-off-by: Anthony PERARD 
> 
> This in principle could have my R-b, but ...
> 
> > --- a/xen/Makefile
> > +++ b/xen/Makefile
> > @@ -90,6 +90,9 @@ _install: $(TARGET)$(CONFIG_XEN_INSTALL_SUFFIX)
> >  .PHONY: _tests
> >  _tests:
> > $(MAKE) -f $(BASEDIR)/Rules.mk -C test tests
> > +.PHONY: install-tests
> > +install-tests:
> > +   $(MAKE) -f $(BASEDIR)/Rules.mk -C test install
> 
> ... I'm irritated by the patch context here: Patch 8 changed
> _tests to tests, and by the numbering this patch goes on top
> of patch 8. Could you clarify what's going on here, please?

I wanted to have this patch earlier in the series. I could probably have
put in the subject something like "[PATCH 1.5/12]" to make this clearer.

Cheers,

-- 
Anthony PERARD

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 8/8] x86/setup: lift dom0 creation out into create_dom0() function

[Julien Grall]

On 01/02/2020 00:33, David Woodhouse wrote:

  if ( xen_cpuidle )
  xen_processor_pmbits |= XEN_PROCESSOR_PM_CX;
  
+printk("%sNX (Execute Disable) protection %sactive\n",

+   cpu_has_nx ? XENLOG_INFO : XENLOG_WARNING "Warning: ",
+   cpu_has_nx ? "" : "not ");
+


The placement of printk shouldn't matter but the change feels a bit 
out-of-context. Would you mind to explain it in the commit message?



  initrdidx = find_first_bit(module_map, mbi->mods_count);
  if ( initrdidx < mbi->mods_count )
  initrd = mod + initrdidx;
@@ -1801,34 +1836,14 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 "Multiple initrd candidates, picking module #%u\n",
 initrdidx);
  
-/*

- * Temporarily clear SMAP in CR4 to allow user-accesses in 
construct_dom0().
- * This saves a large number of corner cases interactions with
- * copy_from_user().
- */
-if ( cpu_has_smap )
-{
-cr4_pv32_mask &= ~X86_CR4_SMAP;
-write_cr4(read_cr4() & ~X86_CR4_SMAP);
-}
-
-printk("%sNX (Execute Disable) protection %sactive\n",
-   cpu_has_nx ? XENLOG_INFO : XENLOG_WARNING "Warning: ",
-   cpu_has_nx ? "" : "not ");
-


[...]


Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v6 04/11] x86/hypervisor: provide hypervisor_fixup_e820

[Roger Pau Monné]

On Fri, Jan 31, 2020 at 05:49:23PM +, Wei Liu wrote:
> And implement the hook for Xen guest.
> 
> Signed-off-by: Wei Liu 
> ---
>  xen/arch/x86/e820.c| 4 ++--
>  xen/arch/x86/guest/hypervisor.c| 6 ++
>  xen/arch/x86/guest/xen/xen.c   | 7 +++
>  xen/include/asm-x86/guest/hypervisor.h | 6 ++
>  4 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/x86/e820.c b/xen/arch/x86/e820.c
> index 3892c9cfb7..2219c63861 100644
> --- a/xen/arch/x86/e820.c
> +++ b/xen/arch/x86/e820.c
> @@ -690,8 +690,8 @@ unsigned long __init init_e820(const char *str, struct 
> e820map *raw)
>  
>  machine_specific_memory_setup(raw);
>  
> -if ( pv_shim )
> -pv_shim_fixup_e820(&e820);
> +if ( cpu_has_hypervisor )
> +hypervisor_e820_fixup(&e820);
>  
>  printk("%s RAM map:\n", str);
>  print_e820_memory_map(e820.map, e820.nr_map);
> diff --git a/xen/arch/x86/guest/hypervisor.c b/xen/arch/x86/guest/hypervisor.c
> index e72c92ffdf..5fd433c8d4 100644
> --- a/xen/arch/x86/guest/hypervisor.c
> +++ b/xen/arch/x86/guest/hypervisor.c
> @@ -66,6 +66,12 @@ void hypervisor_resume(void)
>  ops->resume();
>  }
>  
> +void __init hypervisor_e820_fixup(struct e820map *e820)
> +{
> +if ( ops && ops->e820_fixup )
> +ops->e820_fixup(e820);
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/arch/x86/guest/xen/xen.c b/xen/arch/x86/guest/xen/xen.c
> index d50f86bae7..45e54dfbba 100644
> --- a/xen/arch/x86/guest/xen/xen.c
> +++ b/xen/arch/x86/guest/xen/xen.c
> @@ -316,11 +316,18 @@ static void resume(void)
>  pv_console_init();
>  }
>  
> +static void __init e820_fixup(struct e820map *e820)
> +{
> +if ( pv_shim )
> +pv_shim_fixup_e820(e820);
> +}
> +
>  static const struct hypervisor_ops ops = {
>  .name = "Xen",
>  .setup = setup,
>  .ap_setup = ap_setup,
>  .resume = resume,
> +.e820_fixup = e820_fixup,
>  };
>  
>  const struct hypervisor_ops *__init xg_probe(void)
> diff --git a/xen/include/asm-x86/guest/hypervisor.h 
> b/xen/include/asm-x86/guest/hypervisor.h
> index b503854c5b..b66cb28333 100644
> --- a/xen/include/asm-x86/guest/hypervisor.h
> +++ b/xen/include/asm-x86/guest/hypervisor.h
> @@ -19,6 +19,8 @@
>  #ifndef __X86_HYPERVISOR_H__
>  #define __X86_HYPERVISOR_H__
>  
> +#include 
> +
>  struct hypervisor_ops {
>  /* Name of the hypervisor */
>  const char *name;
> @@ -28,6 +30,8 @@ struct hypervisor_ops {
>  int (*ap_setup)(void);
>  /* Resume from suspension */
>  void (*resume)(void);
> +/* Fix up e820 map */
> +void (*e820_fixup)(struct e820map *e820);
>  };
>  
>  #ifdef CONFIG_GUEST
> @@ -36,6 +40,7 @@ const char *hypervisor_probe(void);
>  void hypervisor_setup(void);
>  int hypervisor_ap_setup(void);
>  void hypervisor_resume(void);
> +void hypervisor_e820_fixup(struct e820map *e820);
>  
>  #else
>  
> @@ -46,6 +51,7 @@ static inline const char *hypervisor_probe(void) { return 
> NULL; }
>  static inline void hypervisor_setup(void) { ASSERT_UNREACHABLE(); }
>  static inline int hypervisor_ap_setup(void) { ASSERT_UNREACHABLE(); return 
> 0; }
>  static inline void hypervisor_resume(void) { ASSERT_UNREACHABLE(); }
> +static inline void hypervisor_e820_fixup(struct e820map *e820) { 
> ASSERT_UNREACHABLE(); }

Are you sure the assert here is fine?

Consider Xen running nested on another hypervisor, and built without
CONFIG_GUEST enabled, I think the above assert would trigger.

Thanks, Roger.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2.1 15/12] squash! xen/build: have the root Makefile generates the CFLAGS

[Anthony PERARD]

On Thu, Jan 30, 2020 at 02:32:13PM +0100, Jan Beulich wrote:
> On 21.01.2020 14:59, Anthony PERARD wrote:
> > The XEN_BUILD_EFI tests in arch/x86/Makefile was filtering out
> > CFLAGS-y, but according to dd40177c1bc8 ("x86-64/EFI: add CFLAGS to
> > check compile"), it was done to filter out -MF. XEN_CFLAGS doesn't
> > have those flags anymore, so no filtering is needed.
> 
> But this should then be part of patch 10, not 12, I would think.

Sound good, I'll check if that's fine.

Thanks,

-- 
Anthony PERARD

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v6 04/11] x86/hypervisor: provide hypervisor_fixup_e820

[Wei Liu]

On Mon, Feb 03, 2020 at 03:32:01PM +0100, Roger Pau Monné wrote:
[...]
> >  
> >  #else
> >  
> > @@ -46,6 +51,7 @@ static inline const char *hypervisor_probe(void) { return 
> > NULL; }
> >  static inline void hypervisor_setup(void) { ASSERT_UNREACHABLE(); }
> >  static inline int hypervisor_ap_setup(void) { ASSERT_UNREACHABLE(); return 
> > 0; }
> >  static inline void hypervisor_resume(void) { ASSERT_UNREACHABLE(); }
> > +static inline void hypervisor_e820_fixup(struct e820map *e820) { 
> > ASSERT_UNREACHABLE(); }
> 
> Are you sure the assert here is fine?
> 
> Consider Xen running nested on another hypervisor, and built without
> CONFIG_GUEST enabled, I think the above assert would trigger.

Hmm... yes, this assertion should be dropped. Thanks.

Wei.

> 
> Thanks, Roger.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 0/4] AMD/IOMMU: Cleanup

[Andrew Cooper]

Various bits of cleanup without any major functional change.  The eventual
plan is to purge the {get,set}_field_{from,in}_u32() helpers, which are
responsible for very hard to follow code, and poor code generation.

Andrew Cooper (4):
  AMD/IOMMU: Move headers to be local
  AMD/IOMMU: Delete iommu_{get,set,clear}_bit() helpers
  AMD/IOMMU: Treat guest head/tail pointers as byte offsets
  AMD/IOMMU: Treat head/tail pointers as byte offsets

 xen/arch/x86/mm/p2m.c  |   1 -
 .../passthrough/amd/iommu-defs.h}  |  41 ++--
 .../passthrough/amd/iommu.h}   | 206 +
 xen/drivers/passthrough/amd/iommu_acpi.c   |   7 +-
 xen/drivers/passthrough/amd/iommu_cmd.c|  32 ++--
 xen/drivers/passthrough/amd/iommu_detect.c |   7 +-
 xen/drivers/passthrough/amd/iommu_guest.c  | 132 ++---
 xen/drivers/passthrough/amd/iommu_init.c   |  64 +++
 xen/drivers/passthrough/amd/iommu_intr.c   |   9 +-
 xen/drivers/passthrough/amd/iommu_map.c|   8 +-
 xen/drivers/passthrough/amd/pci_amd_iommu.c|   9 +-
 xen/include/asm-x86/amd-iommu.h| 190 ---
 12 files changed, 298 insertions(+), 408 deletions(-)
 rename xen/{include/asm-x86/hvm/svm/amd-iommu-defs.h => 
drivers/passthrough/amd/iommu-defs.h} (92%)
 rename xen/{include/asm-x86/hvm/svm/amd-iommu-proto.h => 
drivers/passthrough/amd/iommu.h} (68%)
 delete mode 100644 xen/include/asm-x86/amd-iommu.h

-- 
2.11.0


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 1/4] AMD/IOMMU: Move headers to be local

[Andrew Cooper]

We currently have amd-iommu-defs.h, amd-iommu-proto.h and amd-iommu.h, and no
references outside of the AMD IOMMU driver.

Keep iommu-defs.h as is, but merge amd-iommu.h and amd-iommu-proto.h to just
iommu.h, and move them both into drivers/passthrough/amd/.  (While merging,
drop the bogus #pragma pack around the *_entry structures.)

Take the opportunity to trim the include lists, including x86/mm/p2m.c
which (AFAICT) hasn't needed this include since c/s aef3f2275 "x86/mm/p2m:
break into common, pt-implementation and pod parts" in 2011.

No functional change.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Wei Liu 
CC: Roger Pau Monné 
CC: George Dunlap 
---
 xen/arch/x86/mm/p2m.c  |   1 -
 .../passthrough/amd/iommu-defs.h}  |   6 +-
 .../passthrough/amd/iommu.h}   | 179 ++-
 xen/drivers/passthrough/amd/iommu_acpi.c   |   7 +-
 xen/drivers/passthrough/amd/iommu_cmd.c|   4 +-
 xen/drivers/passthrough/amd/iommu_detect.c |   7 +-
 xen/drivers/passthrough/amd/iommu_guest.c  |   4 +-
 xen/drivers/passthrough/amd/iommu_init.c   |  13 +-
 xen/drivers/passthrough/amd/iommu_intr.c   |   9 +-
 xen/drivers/passthrough/amd/iommu_map.c|   8 +-
 xen/drivers/passthrough/amd/pci_amd_iommu.c|   9 +-
 xen/include/asm-x86/amd-iommu.h| 190 -
 12 files changed, 194 insertions(+), 243 deletions(-)
 rename xen/{include/asm-x86/hvm/svm/amd-iommu-defs.h => 
drivers/passthrough/amd/iommu-defs.h} (99%)
 rename xen/{include/asm-x86/hvm/svm/amd-iommu-proto.h => 
drivers/passthrough/amd/iommu.h} (70%)
 delete mode 100644 xen/include/asm-x86/amd-iommu.h

diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index def13f657b..fd9f09536d 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -38,7 +38,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
diff --git a/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h 
b/xen/drivers/passthrough/amd/iommu-defs.h
similarity index 99%
rename from xen/include/asm-x86/hvm/svm/amd-iommu-defs.h
rename to xen/drivers/passthrough/amd/iommu-defs.h
index 78368f16d9..f8b62cb033 100644
--- a/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h
+++ b/xen/drivers/passthrough/amd/iommu-defs.h
@@ -17,8 +17,8 @@
  * along with this program; If not, see .
  */
 
-#ifndef _ASM_X86_64_AMD_IOMMU_DEFS_H
-#define _ASM_X86_64_AMD_IOMMU_DEFS_H
+#ifndef AMD_IOMMU_DEFS_H
+#define AMD_IOMMU_DEFS_H
 
 /* IOMMU Command Buffer entries: in power of 2 increments, minimum of 256 */
 #define IOMMU_CMD_BUFFER_DEFAULT_ENTRIES   512
@@ -506,7 +506,7 @@ struct amd_iommu_pte {
 #define IOMMU_REG_BASE_ADDR_HIGH_MASK   0x000F
 #define IOMMU_REG_BASE_ADDR_HIGH_SHIFT  0
 
-#endif /* _ASM_X86_64_AMD_IOMMU_DEFS_H */
+#endif /* AMD_IOMMU_DEFS_H */
 
 /*
  * Local variables:
diff --git a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h 
b/xen/drivers/passthrough/amd/iommu.h
similarity index 70%
rename from xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
rename to xen/drivers/passthrough/amd/iommu.h
index b5c0d50119..f590de8cbf 100644
--- a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
+++ b/xen/drivers/passthrough/amd/iommu.h
@@ -16,15 +16,180 @@
  * You should have received a copy of the GNU General Public License
  * along with this program; If not, see .
  */
-
-#ifndef _ASM_X86_64_AMD_IOMMU_PROTO_H
-#define _ASM_X86_64_AMD_IOMMU_PROTO_H
-
+#ifndef AMD_IOMMU_H
+#define AMD_IOMMU_H
+
+#include 
+#include 
+#include 
+#include 
+#include 
 #include 
-#include 
-#include 
 #include 
 
+#include 
+#include 
+
+#include "iommu-defs.h"
+
+#define iommu_found()   (!list_empty(&amd_iommu_head))
+
+extern struct list_head amd_iommu_head;
+
+typedef struct event_entry
+{
+uint32_t data[4];
+} event_entry_t;
+
+typedef struct ppr_entry
+{
+uint32_t data[4];
+} ppr_entry_t;
+
+typedef struct cmd_entry
+{
+uint32_t data[4];
+} cmd_entry_t;
+
+struct table_struct {
+void *buffer;
+unsigned long entries;
+unsigned long alloc_size;
+};
+
+struct ring_buffer {
+void *buffer;
+unsigned long entries;
+unsigned long alloc_size;
+uint32_t tail;
+uint32_t head;
+spinlock_t lock;/* protect buffer pointers */
+};
+
+typedef struct iommu_cap {
+uint32_t header;/* offset 00h */
+uint32_t base_low;  /* offset 04h */
+uint32_t base_hi;   /* offset 08h */
+uint32_t range; /* offset 0Ch */
+uint32_t misc;  /* offset 10h */
+} iommu_cap_t;
+
+struct amd_iommu {
+struct list_head list;
+spinlock_t lock; /* protect iommu */
+
+u16 seg;
+u16 bdf;
+struct msi_desc msi;
+
+u16 cap_offset;
+iommu_cap_t cap;
+
+u8 ht_flags;
+union amd_iommu_ext_features features;
+

[Xen-devel] [PATCH 3/4] AMD/IOMMU: Treat guest head/tail pointers as byte offsets

[Andrew Cooper]

The MMIO registers as already byte offsets.  By masking out the reserved bits
suitably in guest_iommu_mmio_write64(), we can use the values directly,
instead of masking/shifting on every use.

Store the buffer size, rather than the number of entries, to keep the same
units for comparison purposes.

This simplifies guest_iommu_get_table_mfn() by dropping the entry_size
parameter, and simplifies the map_domain_page() handling by being able to drop
the log_base variables.

No functional change.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Wei Liu 
CC: Roger Pau Monné 

Untested - this is all dead code, and there are plenty of security issues
which need adjusting before it can start being used again.
---
 xen/drivers/passthrough/amd/iommu.h   |  2 +-
 xen/drivers/passthrough/amd/iommu_guest.c | 85 +++
 2 files changed, 42 insertions(+), 45 deletions(-)

diff --git a/xen/drivers/passthrough/amd/iommu.h 
b/xen/drivers/passthrough/amd/iommu.h
index 81b6812d3a..0b598d06f8 100644
--- a/xen/drivers/passthrough/amd/iommu.h
+++ b/xen/drivers/passthrough/amd/iommu.h
@@ -152,7 +152,7 @@ struct guest_buffer {
 struct mmio_reg reg_base;
 struct mmio_reg reg_tail;
 struct mmio_reg reg_head;
-uint32_tentries;
+uint32_tsize;
 };
 
 struct guest_iommu_msi {
diff --git a/xen/drivers/passthrough/amd/iommu_guest.c 
b/xen/drivers/passthrough/amd/iommu_guest.c
index d05901d348..ef9c4a4d29 100644
--- a/xen/drivers/passthrough/amd/iommu_guest.c
+++ b/xen/drivers/passthrough/amd/iommu_guest.c
@@ -103,14 +103,13 @@ static void guest_iommu_deliver_msi(struct domain *d)
 
 static unsigned long guest_iommu_get_table_mfn(struct domain *d,
uint64_t base_raw,
-   unsigned int entry_size,
unsigned int pos)
 {
 unsigned long idx, gfn, mfn;
 p2m_type_t p2mt;
 
 gfn = get_gfn_from_base_reg(base_raw);
-idx = (pos * entry_size) >> PAGE_SHIFT;
+idx = pos >> PAGE_SHIFT;
 
 mfn = mfn_x(get_gfn(d, gfn + idx, &p2mt));
 put_gfn(d, gfn);
@@ -133,14 +132,14 @@ static void guest_iommu_enable_ring_buffer(struct 
guest_iommu *iommu,
 uint32_t length_raw = get_field_from_reg_u32(buffer->reg_base.hi,
  RING_BF_LENGTH_MASK,
  RING_BF_LENGTH_SHIFT);
-buffer->entries = 1 << length_raw;
+buffer->size = entry_size << length_raw;
 }
 
 void guest_iommu_add_ppr_log(struct domain *d, u32 entry[])
 {
 uint16_t gdev_id;
 unsigned long mfn, tail, head;
-ppr_entry_t *log, *log_base;
+ppr_entry_t *log;
 struct guest_iommu *iommu;
 
 if ( !is_hvm_domain(d) )
@@ -150,10 +149,10 @@ void guest_iommu_add_ppr_log(struct domain *d, u32 
entry[])
 if ( !iommu )
 return;
 
-tail = iommu_get_rb_pointer(iommu->ppr_log.reg_tail.lo);
-head = iommu_get_rb_pointer(iommu->ppr_log.reg_head.lo);
+tail = iommu->ppr_log.reg_tail.lo;
+head = iommu->ppr_log.reg_head.lo;
 
-if ( tail >= iommu->ppr_log.entries || head >= iommu->ppr_log.entries )
+if ( tail >= iommu->ppr_log.size || head >= iommu->ppr_log.size )
 {
 AMD_IOMMU_DEBUG("Error: guest iommu ppr log overflows\n");
 guest_iommu_disable(iommu);
@@ -161,11 +160,10 @@ void guest_iommu_add_ppr_log(struct domain *d, u32 
entry[])
 }
 
 mfn = guest_iommu_get_table_mfn(d, reg_to_u64(iommu->ppr_log.reg_base),
-sizeof(ppr_entry_t), tail);
+tail);
 ASSERT(mfn_valid(_mfn(mfn)));
 
-log_base = map_domain_page(_mfn(mfn));
-log = log_base + tail % (PAGE_SIZE / sizeof(ppr_entry_t));
+log = map_domain_page(_mfn(mfn)) + (tail & PAGE_MASK);
 
 /* Convert physical device id back into virtual device id */
 gdev_id = guest_bdf(d, iommu_get_devid_from_cmd(entry[0]));
@@ -174,13 +172,15 @@ void guest_iommu_add_ppr_log(struct domain *d, u32 
entry[])
 memcpy(log, entry, sizeof(ppr_entry_t));
 
 /* Now shift ppr log tail pointer */
-if ( ++tail >= iommu->ppr_log.entries )
+tail += sizeof(ppr_entry_t);
+if ( tail >= iommu->ppr_log.size )
 {
 tail = 0;
 iommu->reg_status.lo |= IOMMU_STATUS_PPR_LOG_OVERFLOW;
 }
-iommu_set_rb_pointer(&iommu->ppr_log.reg_tail.lo, tail);
-unmap_domain_page(log_base);
+
+iommu->ppr_log.reg_tail.lo = tail;
+unmap_domain_page(log);
 
 guest_iommu_deliver_msi(d);
 }
@@ -189,7 +189,7 @@ void guest_iommu_add_event_log(struct domain *d, u32 
entry[])
 {
 uint16_t dev_id;
 unsigned long mfn, tail, head;
-event_entry_t *log, *log_base;
+event_entry_t *log;
 struct guest_iommu *iommu;
 
 if ( !is_hvm_domain(d) )
@@ -199,10 +199,10 @@ void guest_iommu_add_event_log(struct domain *d, u32 
ent

[Xen-devel] [PATCH 2/4] AMD/IOMMU: Delete iommu_{get, set, clear}_bit() helpers

[Andrew Cooper]

These are obfuscations around simple bit operations, and the compiler really
can do a better job when it can see them normally:

  add/remove: 0/0 grow/shrink: 0/5 up/down: 0/-181 (-181)
  Function old new   delta
  guest_iommu_add_ppr_log  266 251 -15
  guest_iommu_add_event_log266 251 -15
  iommu_reset_log  274 250 -24
  guest_iommu_process_command 16021544 -58
  guest_iommu_mmio_write  11231054 -69
  Total: Before=3014099, After=3013918, chg -0.01%

Drop all status register MASK/SHIFT constants, and enumerate the bits
normally.  Rename EVENT_OVERFLOW to EVENT_LOG_OVERFLOW for consistency.  (The
field name in the spec is inconsistent, despite the description referring to
an overflow of the event log.)

The only semantic change is in iommu_reset_log() where 'run_bit' changes from
being a bit position to being a single-bit mask.

No functional change.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Wei Liu 
CC: Roger Pau Monné 
---
 xen/drivers/passthrough/amd/iommu-defs.h  | 34 +---
 xen/drivers/passthrough/amd/iommu.h   | 15 ---
 xen/drivers/passthrough/amd/iommu_cmd.c   |  8 +++---
 xen/drivers/passthrough/amd/iommu_guest.c | 43 +--
 xen/drivers/passthrough/amd/iommu_init.c  | 21 +++
 5 files changed, 43 insertions(+), 78 deletions(-)

diff --git a/xen/drivers/passthrough/amd/iommu-defs.h 
b/xen/drivers/passthrough/amd/iommu-defs.h
index f8b62cb033..963009de6a 100644
--- a/xen/drivers/passthrough/amd/iommu-defs.h
+++ b/xen/drivers/passthrough/amd/iommu-defs.h
@@ -437,28 +437,18 @@ union amd_iommu_x2apic_control {
 
 /* Status Register*/
 #define IOMMU_STATUS_MMIO_OFFSET   0x2020
-#define IOMMU_STATUS_EVENT_OVERFLOW_MASK   0x0001
-#define IOMMU_STATUS_EVENT_OVERFLOW_SHIFT  0
-#define IOMMU_STATUS_EVENT_LOG_INT_MASK0x0002
-#define IOMMU_STATUS_EVENT_LOG_INT_SHIFT   1
-#define IOMMU_STATUS_COMP_WAIT_INT_MASK0x0004
-#define IOMMU_STATUS_COMP_WAIT_INT_SHIFT   2
-#define IOMMU_STATUS_EVENT_LOG_RUN_MASK0x0008
-#define IOMMU_STATUS_EVENT_LOG_RUN_SHIFT   3
-#define IOMMU_STATUS_CMD_BUFFER_RUN_MASK   0x0010
-#define IOMMU_STATUS_CMD_BUFFER_RUN_SHIFT  4
-#define IOMMU_STATUS_PPR_LOG_OVERFLOW_MASK  0x0020
-#define IOMMU_STATUS_PPR_LOG_OVERFLOW_SHIFT 5
-#define IOMMU_STATUS_PPR_LOG_INT_MASK   0x0040
-#define IOMMU_STATUS_PPR_LOG_INT_SHIFT  6
-#define IOMMU_STATUS_PPR_LOG_RUN_MASK   0x0080
-#define IOMMU_STATUS_PPR_LOG_RUN_SHIFT  7
-#define IOMMU_STATUS_GAPIC_LOG_OVERFLOW_MASK0x0100
-#define IOMMU_STATUS_GAPIC_LOG_OVERFLOW_SHIFT   8
-#define IOMMU_STATUS_GAPIC_LOG_INT_MASK 0x0200
-#define IOMMU_STATUS_GAPIC_LOG_INT_SHIFT9
-#define IOMMU_STATUS_GAPIC_LOG_RUN_MASK 0x0400
-#define IOMMU_STATUS_GAPIC_LOG_RUN_SHIFT10
+
+#define IOMMU_STATUS_EVENT_LOG_OVERFLOW   0x0001
+#define IOMMU_STATUS_EVENT_LOG_INT0x0002
+#define IOMMU_STATUS_COMP_WAIT_INT0x0004
+#define IOMMU_STATUS_EVENT_LOG_RUN0x0008
+#define IOMMU_STATUS_CMD_BUFFER_RUN   0x0010
+#define IOMMU_STATUS_PPR_LOG_OVERFLOW 0x0020
+#define IOMMU_STATUS_PPR_LOG_INT  0x0040
+#define IOMMU_STATUS_PPR_LOG_RUN  0x0080
+#define IOMMU_STATUS_GAPIC_LOG_OVERFLOW   0x0100
+#define IOMMU_STATUS_GAPIC_LOG_INT0x0200
+#define IOMMU_STATUS_GAPIC_LOG_RUN0x0400
 
 /* I/O Page Table */
 #define IOMMU_PAGE_TABLE_ENTRY_SIZE8
diff --git a/xen/drivers/passthrough/amd/iommu.h 
b/xen/drivers/passthrough/amd/iommu.h
index f590de8cbf..81b6812d3a 100644
--- a/xen/drivers/passthrough/amd/iommu.h
+++ b/xen/drivers/passthrough/amd/iommu.h
@@ -374,21 +374,6 @@ static inline void __free_amd_iommu_tables(void *table, 
int order)
 free_xenheap_pages(table, order);
 }
 
-static inline void iommu_set_bit(uint32_t *reg, uint32_t bit)
-{
-set_field_in_reg_u32(IOMMU_CONTROL_ENABLED, *reg, 1U << bit, bit, reg);
-}
-
-static inline void iommu_clear_bit(uint32_t *reg, uint32_t bit)
-{
-set_field_in_reg_u32(IOMMU_CONTROL_DISABLED, *reg, 1U << bit, bit, reg);
-}
-
-static inline uint32_t iommu_get_bit(uint32_t reg, uint32_t bit)
-{
-return get_field_from_reg_u32(reg, 1U << bit, bit);
-}
-
 static inline int iommu_has_cap(struct amd_iommu *iommu, uint32_t bit)
 {
 return !!(iommu->cap.header & (1u << bit));
diff --git a/xen/drivers/passthrough/amd/iommu_cmd.c 
b/xen/drivers/passthrough/amd/iommu_cmd.c
index 92eaab407b..166f0e7263 100644
--- a/xen/drivers/passthrough/amd/iommu_cmd.c
+++ b/xen/drivers/passthrough/amd/iommu_cmd.c
@@ -68,7 +68,7 @@ static void flush_command_buffer(struct amd_iommu *iommu)
 int loop_count, 

[Xen-devel] [PATCH 4/4] AMD/IOMMU: Treat head/tail pointers as byte offsets

[Andrew Cooper]

The MMIO registers as already byte offsets.  Using them in this form removes
the need to shift their values for use.

It is also inefficient to store both entries and alloc_size (which differ by a
fixed entry_size).  Rename alloc_size to size, and drop entries entirely,
which simplifies the allocation/deallocation helpers slightly.

Mark send_iommu_command() and invalidate_iommu_all() as static, as they have
no external declaration or callers.

Overall, this simplification allows GCC to optimise sufficiently to construct
commands directly in the command ring, rather than on the stack and copying
them into place.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Wei Liu 
CC: Roger Pau Monné 

Tested on a Rome system.
---
 xen/drivers/passthrough/amd/iommu-defs.h |  1 -
 xen/drivers/passthrough/amd/iommu.h  | 18 ++
 xen/drivers/passthrough/amd/iommu_cmd.c  | 20 
 xen/drivers/passthrough/amd/iommu_init.c | 30 +-
 4 files changed, 23 insertions(+), 46 deletions(-)

diff --git a/xen/drivers/passthrough/amd/iommu-defs.h 
b/xen/drivers/passthrough/amd/iommu-defs.h
index 963009de6a..50613ca150 100644
--- a/xen/drivers/passthrough/amd/iommu-defs.h
+++ b/xen/drivers/passthrough/amd/iommu-defs.h
@@ -481,7 +481,6 @@ struct amd_iommu_pte {
 #define INV_IOMMU_ALL_PAGES_ADDRESS  ((1ULL << 63) - 1)
 
 #define IOMMU_RING_BUFFER_PTR_MASK  0x0007FFF0
-#define IOMMU_RING_BUFFER_PTR_SHIFT 4
 
 #define IOMMU_CMD_DEVICE_ID_MASK0x
 #define IOMMU_CMD_DEVICE_ID_SHIFT   0
diff --git a/xen/drivers/passthrough/amd/iommu.h 
b/xen/drivers/passthrough/amd/iommu.h
index 0b598d06f8..1abfdc685a 100644
--- a/xen/drivers/passthrough/amd/iommu.h
+++ b/xen/drivers/passthrough/amd/iommu.h
@@ -58,12 +58,11 @@ struct table_struct {
 };
 
 struct ring_buffer {
+spinlock_t lock;/* protect buffer pointers */
 void *buffer;
-unsigned long entries;
-unsigned long alloc_size;
 uint32_t tail;
 uint32_t head;
-spinlock_t lock;/* protect buffer pointers */
+uint32_t size;
 };
 
 typedef struct iommu_cap {
@@ -379,19 +378,6 @@ static inline int iommu_has_cap(struct amd_iommu *iommu, 
uint32_t bit)
 return !!(iommu->cap.header & (1u << bit));
 }
 
-/* access tail or head pointer of ring buffer */
-static inline uint32_t iommu_get_rb_pointer(uint32_t reg)
-{
-return get_field_from_reg_u32(reg, IOMMU_RING_BUFFER_PTR_MASK,
-  IOMMU_RING_BUFFER_PTR_SHIFT);
-}
-
-static inline void iommu_set_rb_pointer(uint32_t *reg, uint32_t val)
-{
-set_field_in_reg_u32(val, *reg, IOMMU_RING_BUFFER_PTR_MASK,
- IOMMU_RING_BUFFER_PTR_SHIFT, reg);
-}
-
 /* access device id field from iommu cmd */
 static inline uint16_t iommu_get_devid_from_cmd(uint32_t cmd)
 {
diff --git a/xen/drivers/passthrough/amd/iommu_cmd.c 
b/xen/drivers/passthrough/amd/iommu_cmd.c
index 166f0e7263..24ef59d436 100644
--- a/xen/drivers/passthrough/amd/iommu_cmd.c
+++ b/xen/drivers/passthrough/amd/iommu_cmd.c
@@ -24,16 +24,14 @@ static int queue_iommu_command(struct amd_iommu *iommu, u32 
cmd[])
 {
 uint32_t tail, head;
 
-tail = iommu->cmd_buffer.tail;
-if ( ++tail == iommu->cmd_buffer.entries )
+tail = iommu->cmd_buffer.tail + IOMMU_CMD_BUFFER_ENTRY_SIZE;
+if ( tail == iommu->cmd_buffer.size )
 tail = 0;
 
-head = iommu_get_rb_pointer(readl(iommu->mmio_base +
-  IOMMU_CMD_BUFFER_HEAD_OFFSET));
+head = readl(iommu->mmio_base + IOMMU_CMD_BUFFER_HEAD_OFFSET);
 if ( head != tail )
 {
-memcpy(iommu->cmd_buffer.buffer +
-   (iommu->cmd_buffer.tail * IOMMU_CMD_BUFFER_ENTRY_SIZE),
+memcpy(iommu->cmd_buffer.buffer + iommu->cmd_buffer.tail,
cmd, IOMMU_CMD_BUFFER_ENTRY_SIZE);
 
 iommu->cmd_buffer.tail = tail;
@@ -45,13 +43,11 @@ static int queue_iommu_command(struct amd_iommu *iommu, u32 
cmd[])
 
 static void commit_iommu_command_buffer(struct amd_iommu *iommu)
 {
-u32 tail = 0;
-
-iommu_set_rb_pointer(&tail, iommu->cmd_buffer.tail);
-writel(tail, iommu->mmio_base+IOMMU_CMD_BUFFER_TAIL_OFFSET);
+writel(iommu->cmd_buffer.tail,
+   iommu->mmio_base + IOMMU_CMD_BUFFER_TAIL_OFFSET);
 }
 
-int send_iommu_command(struct amd_iommu *iommu, u32 cmd[])
+static int send_iommu_command(struct amd_iommu *iommu, u32 cmd[])
 {
 if ( queue_iommu_command(iommu, cmd) )
 {
@@ -261,7 +257,7 @@ static void invalidate_interrupt_table(struct amd_iommu 
*iommu, u16 device_id)
 send_iommu_command(iommu, cmd);
 }
 
-void invalidate_iommu_all(struct amd_iommu *iommu)
+static void invalidate_iommu_all(struct amd_iommu *iommu)
 {
 u32 cmd[4], entry;
 
diff --git a/xen/drivers/passthrough/amd/iommu_init.c 
b/xen/drivers/passthrough/amd/iommu_init.c
index 7bf6fef3ee..cfdeeefc2d 100644
--- a/xen/drivers/passthrough/amd/iomm

Re: [Xen-devel] [PATCH v6 05/11] x86/hyperv: setup hypercall page

[Roger Pau Monné]

On Fri, Jan 31, 2020 at 05:49:24PM +, Wei Liu wrote:
> Hyper-V uses a technique called overlay page for its hypercall page. It
> will insert a backing page to the guest when the hypercall functionality
> is enabled. That means we can use a page that is not backed by real
> memory for hypercall page.
> 
> Use the top-most addressable page for that purpose. Adjust e820 map
> accordingly.

Can you add this is done to avoid page shattering and to make sure
Xen isn't overwriting any MMIO area which might be present at lower
addresses?

> 
> We also need to register Xen's guest OS ID to Hyper-V. Use 0x3 as the
> vendor ID. Fix the comment in hyperv-tlfs.h while at it.
> 
> Signed-off-by: Wei Liu 
> ---
> v6:
> 1. Use hv_guest_os_id
> 2. Use new e820_fixup hook
> 3. Add a BUILD_BUG_ON
> 
> v5:
> 1. use hypervisor_reserve_top_pages
> 2. add a macro for hypercall page mfn
> 3. address other misc comments
> 
> v4:
> 1. Use fixmap
> 2. Follow routines listed in TLFS
> ---
>  xen/arch/x86/guest/hyperv/hyperv.c  | 69 +++--
>  xen/include/asm-x86/guest/hyperv-tlfs.h |  5 +-
>  xen/include/asm-x86/guest/hyperv.h  |  3 ++
>  3 files changed, 72 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/x86/guest/hyperv/hyperv.c 
> b/xen/arch/x86/guest/hyperv/hyperv.c
> index 8d38313d7a..7c2a96d70e 100644
> --- a/xen/arch/x86/guest/hyperv/hyperv.c
> +++ b/xen/arch/x86/guest/hyperv/hyperv.c
> @@ -19,15 +19,27 @@
>   * Copyright (c) 2019 Microsoft.
>   */
>  #include 
> +#include 
>  
> +#include 
>  #include 
>  #include 
> +#include 
>  
>  struct ms_hyperv_info __read_mostly ms_hyperv;
>  
> -static const struct hypervisor_ops ops = {
> -.name = "Hyper-V",
> -};
> +static uint64_t generate_guest_id(void)
> +{
> +union hv_guest_os_id id;
> +
> +id.vendor = HV_XEN_VENDOR_ID;
> +id.major = xen_major_version();
> +id.minor = xen_minor_version();
> +
> +return id.raw;
> +}
> +
> +static const struct hypervisor_ops ops;
>  
>  const struct hypervisor_ops *__init hyperv_probe(void)
>  {
> @@ -72,6 +84,57 @@ const struct hypervisor_ops *__init hyperv_probe(void)
>  return &ops;
>  }
>  
> +static void __init setup_hypercall_page(void)
> +{
> +union hv_x64_msr_hypercall_contents hypercall_msr;
> +union hv_guest_os_id guest_id;
> +unsigned long mfn;
> +
> +BUILD_BUG_ON(HV_HYP_PAGE_SHIFT != PAGE_SHIFT);
> +
> +rdmsrl(HV_X64_MSR_GUEST_OS_ID, guest_id.raw);
> +if ( !guest_id.raw )
> +{
> +guest_id.raw = generate_guest_id();
> +wrmsrl(HV_X64_MSR_GUEST_OS_ID, guest_id.raw);
> +}
> +
> +rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> +if ( !hypercall_msr.enable )
> +{
> +mfn = HV_HCALL_MFN;
> +hypercall_msr.enable = 1;
> +hypercall_msr.guest_physical_address = mfn;
> +wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> +} else {
> +mfn = hypercall_msr.guest_physical_address;
> +}
> +
> +rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> +BUG_ON(!hypercall_msr.enable);
> +
> +set_fixmap_x(FIX_X_HYPERV_HCALL, mfn << PAGE_SHIFT);
> +}
> +
> +static void __init setup(void)
> +{
> +setup_hypercall_page();
> +}
> +
> +static void __init e820_fixup(struct e820map *e820)
> +{
> +uint64_t s = HV_HCALL_MFN << PAGE_SHIFT;
> +
> +if ( !e820_add_range(e820, s, s + PAGE_SIZE, E820_RESERVED) )

I think end should be s + PAGE_SIZE - 1, or else it expands across two
pages?

Thanks, Roger.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 8/8] x86/setup: lift dom0 creation out into create_dom0() function

[David Woodhouse]

On Mon, 2020-02-03 at 14:28 +, Julien Grall wrote:
> The placement of printk shouldn't matter but the change feels a bit 
> out-of-context. Would you mind to explain it in the commit message?

I didn't really intend to move the printk up; what I intended to do was
move the setting of 'initrd' down, so that it's right before the
create_dom0() call that it is preparing for. Which is purely cosmetic
for now, and more practical later when all that goes into a single
conditional for the non-live-update boot (as shown below).

Will update the commit message to note this; thanks.


printk("%sNX (Execute Disable) protection %sactive\n",
   cpu_has_nx ? XENLOG_INFO : XENLOG_WARNING "Warning: ",
   cpu_has_nx ? "" : "not ");

if ( lu_breadcrumb_phys )
{
dom0 = lu_restore_domains(&lu_stream);
if ( dom0 == NULL )
panic("No DOM0 found in live update data\n");

lu_stream_free(&lu_stream);
}
else
{
initrdidx = find_first_bit(module_map, mbi->mods_count);
if ( initrdidx < mbi->mods_count )
initrd = mod + initrdidx;

if ( bitmap_weight(module_map, mbi->mods_count) > 1 )
printk(XENLOG_WARNING
   "Multiple initrd candidates, picking module #%u\n",
   initrdidx);

/*
 * We're going to setup domain0 using the module(s) that we
 * stashed safely above our heap. The second module, if
 * present, is an initrd ramdisk.
 */
dom0 = create_dom0(mod, modules_headroom, initrd, kextra, loader);
if ( dom0 == NULL )
panic("Could not set up DOM0 guest OS\n");
}


smime.p7s
Description: S/MIME cryptographic signature
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v4 2/7] x86/HVM: rename a variable in __hvm_copy()

[Andrew Cooper]

On 31/01/2020 16:42, Jan Beulich wrote:
> This is to reflect its actual purpose. Also use in a 2nd place.
>
> Signed-off-by: Jan Beulich 

Reviewed-by: Andrew Cooper 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v4 3/7] x86/HVM: introduce "curr" into hvmemul_rep_{mov, sto}s()

[Andrew Cooper]

On 31/01/2020 16:43, Jan Beulich wrote:
> There are a number of uses of "current" already, and more may appear
> down the road. Latch into a local variable.
>
> At this occasion also drop stray casts from code getting touched anyway.
>
> Signed-off-by: Jan Beulich 

Reviewed-by: Andrew Cooper 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v6 05/11] x86/hyperv: setup hypercall page

[Wei Liu]

On Mon, Feb 03, 2020 at 04:01:54PM +0100, Roger Pau Monné wrote:
> On Fri, Jan 31, 2020 at 05:49:24PM +, Wei Liu wrote:
> > Hyper-V uses a technique called overlay page for its hypercall page. It
> > will insert a backing page to the guest when the hypercall functionality
> > is enabled. That means we can use a page that is not backed by real
> > memory for hypercall page.
> > 
> > Use the top-most addressable page for that purpose. Adjust e820 map
> > accordingly.
> 
> Can you add this is done to avoid page shattering and to make sure
> Xen isn't overwriting any MMIO area which might be present at lower
> addresses?

NP.

> 
> > +
> > +static void __init e820_fixup(struct e820map *e820)
> > +{
> > +uint64_t s = HV_HCALL_MFN << PAGE_SHIFT;
> > +
> > +if ( !e820_add_range(e820, s, s + PAGE_SIZE, E820_RESERVED) )
> 
> I think end should be s + PAGE_SIZE - 1, or else it expands across two
> pages?

No, it shouldn't.

E820 entry records the size of the region, which is calculated as
end-start. The one usage in pv/shim.c follows the same pattern here.

Wei.

> 
> Thanks, Roger.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 4/9] xen: add basic hypervisor filesystem support

[Jan Beulich]

On 21.01.2020 09:43, Juergen Gross wrote:
> ---
>  xen/arch/arm/traps.c |   1 +
>  xen/arch/x86/hvm/hypercall.c |   1 +
>  xen/arch/x86/hypercall.c |   1 +
>  xen/arch/x86/pv/hypercall.c  |   1 +
>  xen/common/Makefile  |   1 +
>  xen/common/hypfs.c   | 365 
> +++
>  xen/include/public/hypfs.h   | 124 +++
>  xen/include/public/xen.h |   1 +
>  xen/include/xen/hypercall.h  |   8 +
>  xen/include/xen/hypfs.h  |  89 +++
>  10 files changed, 592 insertions(+)

Even if it's just two structures that you have in the public
header, your assertion of the interface being guest bitness
agnostic should be accompanied by actual proof, i.e. addition
to xen/include/xlat.lst.

> +static int add_entry(struct hypfs_entry_dir *parent, struct hypfs_entry *new)
> +{
> +int ret = -ENOENT;
> +struct hypfs_entry *e;
> +
> +write_lock(&hypfs_lock);
> +
> +list_for_each_entry ( e, &parent->dirlist, list )
> +{
> +int cmp = strcmp(e->name, new->name);
> +
> +if ( cmp > 0 )
> +{
> +ret = 0;
> +list_add_tail(&new->list, &e->list);
> +break;
> +}
> +if ( cmp == 0 )
> +{
> +ret = -EEXIST;
> +break;
> +}
> +}
> +
> +if ( ret == -ENOENT )
> +{
> +ret = 0;
> +list_add_tail(&new->list, &parent->dirlist);
> +}
> +
> +if ( !ret )
> +{
> +unsigned int sz = strlen(new->name) + 1;
> +
> +parent->e.size += DIRENTRY_SIZE(sz);

Would DIRENTRY_SIZE() perhaps better include the "+ 1"?

> +int hypfs_add_entry(struct hypfs_entry_dir *parent,
> +struct hypfs_entry *entry, bool nofault)
> +{
> +int ret;
> +
> +ret = add_entry(parent, entry);
> +BUG_ON(nofault && ret);
> +
> +return ret;
> +}

While this and its two siblings have no caller, the one above
doesn't even have a declaration in the header file. What is
this intended to be used for (from external callers)?

I also think the "nofault" aspect could do with discussing in
the commit message - it seems quite odd to me.

> +static int hypfs_get_path_user(char *buf, XEN_GUEST_HANDLE_PARAM(void) uaddr,
> +   unsigned long len)

For consistency with naming elsewhere as well as uaddr here -
ulen?

> +static struct hypfs_entry *hypfs_get_entry_rel(struct hypfs_entry_dir *dir,
> +   const char *path)
> +{
> +const char *end;
> +struct hypfs_entry *entry;
> +unsigned int name_len;
> +
> +if ( !*path )
> +return &dir->e;
> +
> +if ( dir->e.type != XEN_HYPFS_TYPE_DIR )
> +return NULL;

Don't these two need switching around, to make sure /a/b/c/
doesn't match a non-dir /a/b/c ?

> +end = strchr(path, '/');
> +if ( !end )
> +end = strchr(path, '\0');
> +name_len = end - path;
> +
> +list_for_each_entry ( entry, &dir->dirlist, list )
> +{
> +int cmp = strncmp(path, entry->name, name_len);
> + struct hypfs_entry_dir *d = container_of(entry,

A hard tab slipped in here.

> + struct hypfs_entry_dir, e);
> +
> +if ( cmp < 0 )
> +return NULL;
> +if ( !cmp && strlen(entry->name) == name_len )
> +return *end ? hypfs_get_entry_rel(d, end + 1) : entry;
> +}
> +
> +return NULL;
> +}
> +
> +struct hypfs_entry *hypfs_get_entry(const char *path)
> +{
> +if ( path[0] != '/' )
> +return NULL;
> +
> +return hypfs_get_entry_rel(&hypfs_root, path + 1);
> +}
> +
> +int hypfs_read_dir(const struct hypfs_entry *entry,
> +   XEN_GUEST_HANDLE_PARAM(void) uaddr)
> +{
> +const struct hypfs_entry_dir *d;
> +struct hypfs_entry *e;

const?

> +static int hypfs_read(const struct hypfs_entry *entry,
> +  XEN_GUEST_HANDLE_PARAM(void) uaddr, unsigned long ulen)
> +{
> +struct xen_hypfs_direntry e;
> +long ret = -EINVAL;
> +
> +if ( ulen < sizeof(e) )
> +goto out;
> +
> +e.flags = entry->write ? XEN_HYPFS_WRITEABLE : 0;
> +e.type = entry->type;
> +e.encoding = entry->encoding;
> +e.content_len = entry->size;
> +
> +ret = -EFAULT;
> +if ( copy_to_guest(uaddr, &e, 1) )
> +goto out;
> +
> +ret = 0;
> +if ( ulen < entry->size + sizeof(e) )
> +goto out;

So you return "success" even if the operation didn't complete
successfully. This isn't very nice, plus ...

> +guest_handle_add_offset(uaddr, sizeof(e));
> +
> +ret = entry->read(entry, uaddr);

... how is the caller to know whether direntry was at least
copied if this then fails?

> +int hypfs_write_leaf(struct hypfs_entry_leaf *leaf,
> + XEN_GUEST_HANDLE_PARAM(void) uaddr, unsigned long ulen)
> +{
> +char *buf;
> +int ret;
> +
> +if ( ulen > leaf->e.size )
> +ulen = le

[Xen-devel] [xen-unstable bisection] complete test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm

[osstest service owner]

branch xen-unstable
xenbranch xen-unstable
job test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm
testid debian-hvm-install

Tree: linux git://xenbits.xen.org/linux-pvops.git
Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu git://xenbits.xen.org/qemu-xen.git
Tree: xen git://xenbits.xen.org/xen.git

*** Found and reproduced problem changeset ***

  Bug is in tree:  xen git://xenbits.xen.org/xen.git
  Bug introduced:  75259239d85d6e522c164f1f00ace89bb2dbb3e6
  Bug not present: bc7b5343053c323e6edf71377d983f0b303a9637
  Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/146703/


  commit 75259239d85d6e522c164f1f00ace89bb2dbb3e6
  Author: Paul Durrant 
  Date:   Fri Jan 31 15:01:44 2020 +
  
  libxl_create: make 'soft reset' explicit
  
  The 'soft reset' code path in libxl__domain_make() is currently taken if a
  valid domid is passed into the function. A subsequent patch will enable
  higher levels of the toolstack to determine the domid of newly created or
  restored domains and therefore this criteria for choosing 'soft reset'
  will no longer be usable.
  
  This patch adds an extra boolean option to libxl__domain_make() to specify
  whether it is being invoked in soft reset context and appropriately
  modifies callers to choose the right value. To facilitate this, a new
  'soft_reset' boolean field is added to struct libxl__domain_create_state
  and the 'domid_soft_reset' field is renamed to 'domid' in anticipation of
  its wider remit. For the moment do_domain_create() will always set
  domid to INVALID_DOMID and hence we can add an assertion into
  libxl__domain_create() that, if it is not called in soft reset context,
  the passed in domid is exactly that value.
  
  Whilst in the neighbourhood, some checks of 'restore_fd > -1' have been
  replaced by 'restore_fd >= 0' to be more conventional and consistent with
  checks of 'restore_fd < 0'.
  
  Signed-off-by: Paul Durrant 
  Acked-by: Ian Jackson 


For bisection revision-tuple graph see:
   
http://logs.test-lab.xenproject.org/osstest/results/bisect/xen-unstable/test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm.debian-hvm-install.html
Revision IDs in each graph node refer, respectively, to the Trees above.


Running cs-bisection-step 
--graph-out=/home/logs/results/bisect/xen-unstable/test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm.debian-hvm-install
 --summary-out=tmp/146703.bisection-summary --basis-template=146563 
--blessings=real,real-bisect xen-unstable 
test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm debian-hvm-install
Searching for failure / basis pass:
 146676 fail [host=huxelrebe0] / 146625 [host=chardonnay1] 146611 
[host=elbling1] 146600 [host=fiano0] 146584 [host=fiano1] 146578 [host=pinot0] 
146563 [host=albana0] 146555 [host=debina1] 146543 [host=pinot1] 146534 
[host=huxelrebe1] 146526 [host=albana1] 146514 [host=chardonnay0] 146505 ok.
Failure / basis pass flights: 146676 / 146505
(tree with no url: minios)
(tree with no url: ovmf)
(tree with no url: seabios)
Tree: linux git://xenbits.xen.org/linux-pvops.git
Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu git://xenbits.xen.org/qemu-xen.git
Tree: xen git://xenbits.xen.org/xen.git
Latest c3038e718a19fc596f7b1baba0f83d5146dc7784 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
d0d8ad39ecb51cd7497cd524484fe09f50876798 
933ebad2470a169504799a1d95b8e410bd9847ef 
41d8869003e96d8b7250ad1d0246371d6929aca6
Basis pass c3038e718a19fc596f7b1baba0f83d5146dc7784 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
d0d8ad39ecb51cd7497cd524484fe09f50876798 
933ebad2470a169504799a1d95b8e410bd9847ef 
f190e634daba1a40570700b3e7697d497874c66f
Generating revisions with ./adhoc-revtuple-generator  
git://xenbits.xen.org/linux-pvops.git#c3038e718a19fc596f7b1baba0f83d5146dc7784-c3038e718a19fc596f7b1baba0f83d5146dc7784
 
git://xenbits.xen.org/osstest/linux-firmware.git#c530a75c1e6a472b0eb9558310b518f0dfcd8860-c530a75c1e6a472b0eb9558310b518f0dfcd8860
 
git://xenbits.xen.org/qemu-xen-traditional.git#d0d8ad39ecb51cd7497cd524484fe09f50876798-d0d8ad39ecb51cd7497cd524484fe09f50876798
 git://xenbits.xen.org/qemu-xen.git#933ebad2470a169504799a1d95b8e41\
 0bd9847ef-933ebad2470a169504799a1d95b8e410bd9847ef 
git://xenbits.xen.org/xen.git#f190e634daba1a40570700b3e7697d497874c66f-41d8869003e96d8b7250ad1d0246371d6929aca6
Loaded 5001 nodes in revision graph
Searching for test results:
 146505 pass c3038e718a19fc596f7b1baba0f83d5146dc7784 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
d0d8ad39ecb51cd7497cd524484fe09f50876798 
933ebad2470a169504799a1d95b8e410bd9847ef 
f190e634daba1a40570700b3e7697d497874c66f
 146514 [host=chardonnay0]
 146526 [host=albana1]
 146534 [host=huxelrebe1]
 146543 [host=p

Re: [Xen-devel] [PATCH v3 1/9] xen: add a generic way to include binary files as variables

[Jan Beulich]

On 03.02.2020 15:02, Jürgen Groß wrote:
> On 03.02.20 14:39, Jan Beulich wrote:
>> On 21.01.2020 09:43, Juergen Gross wrote:
>>> --- /dev/null
>>> +++ b/xen/tools/binfile
>>> @@ -0,0 +1,29 @@
>>> +#!/bin/sh
>>> +# usage: binfile [-i]   
>>> +# -i add to .init.rodata (default: .rodata) section
>>> +
>>> +[ "$1" = "-i" ] && {
>>> +shift
>>> +section=".init"
>>> +}
>>> +
>>> +target=$1
>>> +binsource=$2
>>> +varname=$3
>>> +
>>> +cat <$target
>>> +#include 
>>> +
>>> +.section $section.rodata, "a", %progbits
>>> +
>>> +.global $varname
>>> +$varname:
>>> +.incbin "$binsource"
>>> +.Lend:
>>> +
>>> +.type $varname, %object
>>> +.size $varname, . - $varname
>>
>> I'd prefer if you used .Lend here as well.
> 
> Okay.
> 
>> I wonder whether, right from the beginning, there wouldn't better
>> be a way to also request better than byte alignment for such a
>> blob.
> 
> I can add that. What about "-a " for 2^n alignment?

SGTM.

>>> --- a/xen/xsm/flask/Makefile
>>> +++ b/xen/xsm/flask/Makefile
>>> @@ -30,6 +30,9 @@ $(AV_H_FILES): $(AV_H_DEPEND)
>>>   obj-bin-$(CONFIG_XSM_FLASK_POLICY) += flask-policy.o
>>>   flask-policy.o: policy.bin
>>>   
>>> +flask-policy.S: $(XEN_ROOT)/xen/tools/binfile
>>> +   $(XEN_ROOT)/xen/tools/binfile -i $@ policy.bin xsm_flask_init_policy
>>
>> Doesn't objcopy provide a means to convert a binary blob into
>> an ELF object containing the binary data from the input file?
>> If so, why involve the assembler and an intermediate file here?
> 
> I can see how to add a symbol for that purpose using a dedicated section
> for each blob, but how to add the size information of the blob?

Hmm, right. It would be doable, but perhaps indeed not very nice.
Fair enough then.

Jan

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 11/12] xen/build: introduce ccflags-y and CFLAGS_$@

[Jan Beulich]

On 03.02.2020 15:23, Anthony PERARD wrote:
> On Thu, Jan 30, 2020 at 02:39:43PM +0100, Jan Beulich wrote:
>> On 17.01.2020 11:53, Anthony PERARD wrote:
>> I have to admit that I'm also a little puzzled by the naming, no
>> matter that it's taken from Linux. It doesn't really seem to fit
>> CFLAGS/AFLAGS and c_flags/a_flags.
> 
> So I've look into the history of Linux, ccflags-y was introduce to get
> rid of EXTRA_CFLAGS and especially to have the -y part in the name of
> the variable.
> 
> So, instead of ccflags-y and the like, we could use CFLAGS-y in Makefile
> of subdirectories.
> 
> For makefiles in subdir, Linux has:
> CFLAGS_$@
> CFLAGS_REMOVE_$@
> ccflags-y
> subdir-ccflags-y
> so CFLAGS-y would be better (and we can think about the subdir one
> later).

In case this doesn't conflict with uses of CFLAGS-y anywhere else
(not sure if any are left by this point of the series) - sounds
good.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2.1 13/12] Makefile: Fix install-tests

[Jan Beulich]

On 03.02.2020 15:29, Anthony PERARD wrote:
> On Thu, Jan 30, 2020 at 12:37:17PM +0100, Jan Beulich wrote:
>> On 21.01.2020 14:59, Anthony PERARD wrote:
>>> The top-level makefile make uses of internal implementation detail of
>>> the xen build system. Avoid that by creating a new target
>>> "install-tests" in xen/Makefile, and by fixing the top-level Makefile
>>> to not call xen/Rules.mk anymore.
>>>
>>> Signed-off-by: Anthony PERARD 
>>
>> This in principle could have my R-b, but ...
>>
>>> --- a/xen/Makefile
>>> +++ b/xen/Makefile
>>> @@ -90,6 +90,9 @@ _install: $(TARGET)$(CONFIG_XEN_INSTALL_SUFFIX)
>>>  .PHONY: _tests
>>>  _tests:
>>> $(MAKE) -f $(BASEDIR)/Rules.mk -C test tests
>>> +.PHONY: install-tests
>>> +install-tests:
>>> +   $(MAKE) -f $(BASEDIR)/Rules.mk -C test install
>>
>> ... I'm irritated by the patch context here: Patch 8 changed
>> _tests to tests, and by the numbering this patch goes on top
>> of patch 8. Could you clarify what's going on here, please?
> 
> I wanted to have this patch earlier in the series. I could probably have
> put in the subject something like "[PATCH 1.5/12]" to make this clearer.

I see. In which case, as indicated,
Reviewed-by: Jan Beulich 

Jan

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v6 05/11] x86/hyperv: setup hypercall page

[Roger Pau Monné]

On Mon, Feb 03, 2020 at 03:07:24PM +, Wei Liu wrote:
> On Mon, Feb 03, 2020 at 04:01:54PM +0100, Roger Pau Monné wrote:
> > On Fri, Jan 31, 2020 at 05:49:24PM +, Wei Liu wrote:
> > > Hyper-V uses a technique called overlay page for its hypercall page. It
> > > will insert a backing page to the guest when the hypercall functionality
> > > is enabled. That means we can use a page that is not backed by real
> > > memory for hypercall page.
> > > 
> > > Use the top-most addressable page for that purpose. Adjust e820 map
> > > accordingly.
> > 
> > Can you add this is done to avoid page shattering and to make sure
> > Xen isn't overwriting any MMIO area which might be present at lower
> > addresses?
> 
> NP.
> 
> > 
> > > +
> > > +static void __init e820_fixup(struct e820map *e820)
> > > +{
> > > +uint64_t s = HV_HCALL_MFN << PAGE_SHIFT;
> > > +
> > > +if ( !e820_add_range(e820, s, s + PAGE_SIZE, E820_RESERVED) )
> > 
> > I think end should be s + PAGE_SIZE - 1, or else it expands across two
> > pages?
> 
> No, it shouldn't.
> 
> E820 entry records the size of the region, which is calculated as
> end-start. The one usage in pv/shim.c follows the same pattern here.

Hm, I see. I'm not sure this is correct, I think the e820 entry
should look like:

addr = s;
size = PAGE_SIZE - 1;

As ranges on the e820 are inclusive, so if size ends up being
PAGE_SIZE then the entry would expand across two pages.

Anyway, this needs fixing elsewhere, and is out of the scope of this
patch.

Thanks, Roger.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 12/12] xen/build: have the root Makefile generates the CFLAGS

[Jan Beulich]

On 03.02.2020 14:57, Anthony PERARD wrote:
> On Thu, Jan 30, 2020 at 03:33:15PM +0100, Jan Beulich wrote:
>> On 17.01.2020 11:53, Anthony PERARD wrote:
>>> +ifneq ($(CONFIG_CC_IS_CLANG),y)
>>> +# Clang doesn't understand this command line argument, and doesn't appear 
>>> to
>>> +# have an suitable alternative.  The resulting compiled binary does 
>>> function,
>>> +# but has an excessively large symbol table.
>>> +CFLAGS += -Wa,--strip-local-absolute
>>> +endif
>>> +
>>> +AFLAGS-y+= -D__ASSEMBLY__
>>
>> Why not just AFLAGS? I think in a overhaul like what you do,
>> anomalies like this one would better be eliminated. The -y
>> forms should be added into the base variables (like you do ...
> 
> I wanted to avoid too much modification, and mostly want to move the code
> around. So it would be easier to check that the commit doesn't introduce
> errors.  So, if your are fine with patch that move code and modify it,
> I'll fix some of the oddities. (Or I can have another patch for it.)

With such extra adjustments at least briefly mentioned in the
description, folding in is fine as far as I'm concerned.

Jan

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v6 05/11] x86/hyperv: setup hypercall page

[Jan Beulich]

On 03.02.2020 16:21, Roger Pau Monné wrote:
> On Mon, Feb 03, 2020 at 03:07:24PM +, Wei Liu wrote:
>> On Mon, Feb 03, 2020 at 04:01:54PM +0100, Roger Pau Monné wrote:
>>> On Fri, Jan 31, 2020 at 05:49:24PM +, Wei Liu wrote:
 Hyper-V uses a technique called overlay page for its hypercall page. It
 will insert a backing page to the guest when the hypercall functionality
 is enabled. That means we can use a page that is not backed by real
 memory for hypercall page.

 Use the top-most addressable page for that purpose. Adjust e820 map
 accordingly.
>>>
>>> Can you add this is done to avoid page shattering and to make sure
>>> Xen isn't overwriting any MMIO area which might be present at lower
>>> addresses?
>>
>> NP.
>>
>>>
 +
 +static void __init e820_fixup(struct e820map *e820)
 +{
 +uint64_t s = HV_HCALL_MFN << PAGE_SHIFT;
 +
 +if ( !e820_add_range(e820, s, s + PAGE_SIZE, E820_RESERVED) )
>>>
>>> I think end should be s + PAGE_SIZE - 1, or else it expands across two
>>> pages?
>>
>> No, it shouldn't.
>>
>> E820 entry records the size of the region, which is calculated as
>> end-start. The one usage in pv/shim.c follows the same pattern here.
> 
> Hm, I see. I'm not sure this is correct, I think the e820 entry
> should look like:
> 
> addr = s;
> size = PAGE_SIZE - 1;
> 
> As ranges on the e820 are inclusive, so if size ends up being
> PAGE_SIZE then the entry would expand across two pages.

Ranges can sensibly be inclusive only when specified as [start,end]
tuples. (start,size) pairs make no sense for representing
[start,start+size], they only make sense for [start,start+size).
Otherwise, as in your example above, size taken on its own is off
by one (i.e. is rather "last byte" than "size").

Modern Linux, when logging the memory map, indeed subtracts 1 from
the sum of addr and size, to show an inclusive range.

Jan

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 2/5] MdePkg: Allow PcdFSBClock to by Dynamic

[Anthony PERARD]

On Mon, Feb 03, 2020 at 01:34:55AM +, Gao, Liming wrote:
> Anthony:
>   This change is OK to me. But if this PCD is configured as Dynamic, its 
> value will be got from PCD service. This operation will take some time and 
> cause the inaccurate time delay. Have you measured its impact?

No, I haven't. But I don't think it matter in a Xen guest, the APIC timer is
emulated anyway, so reading from a register of the APIC is going to be
slower than getting the value from the PCD services, I think.
(Hopefully, I'm not too wrong.)

But I'll give it at measuring the difference, it would be interesting to
know.

Thanks,

-- 
Anthony PERARD

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v6 05/11] x86/hyperv: setup hypercall page

[Wei Liu]

On Mon, Feb 03, 2020 at 04:32:52PM +0100, Jan Beulich wrote:
> On 03.02.2020 16:21, Roger Pau Monné wrote:
> > On Mon, Feb 03, 2020 at 03:07:24PM +, Wei Liu wrote:
> >> On Mon, Feb 03, 2020 at 04:01:54PM +0100, Roger Pau Monné wrote:
> >>> On Fri, Jan 31, 2020 at 05:49:24PM +, Wei Liu wrote:
>  Hyper-V uses a technique called overlay page for its hypercall page. It
>  will insert a backing page to the guest when the hypercall functionality
>  is enabled. That means we can use a page that is not backed by real
>  memory for hypercall page.
> 
>  Use the top-most addressable page for that purpose. Adjust e820 map
>  accordingly.
> >>>
> >>> Can you add this is done to avoid page shattering and to make sure
> >>> Xen isn't overwriting any MMIO area which might be present at lower
> >>> addresses?
> >>
> >> NP.
> >>
> >>>
>  +
>  +static void __init e820_fixup(struct e820map *e820)
>  +{
>  +uint64_t s = HV_HCALL_MFN << PAGE_SHIFT;
>  +
>  +if ( !e820_add_range(e820, s, s + PAGE_SIZE, E820_RESERVED) )
> >>>
> >>> I think end should be s + PAGE_SIZE - 1, or else it expands across two
> >>> pages?
> >>
> >> No, it shouldn't.
> >>
> >> E820 entry records the size of the region, which is calculated as
> >> end-start. The one usage in pv/shim.c follows the same pattern here.
> > 
> > Hm, I see. I'm not sure this is correct, I think the e820 entry
> > should look like:
> > 
> > addr = s;
> > size = PAGE_SIZE - 1;
> > 
> > As ranges on the e820 are inclusive, so if size ends up being
> > PAGE_SIZE then the entry would expand across two pages.
> 
> Ranges can sensibly be inclusive only when specified as [start,end]
> tuples. (start,size) pairs make no sense for representing
> [start,start+size], they only make sense for [start,start+size).
> Otherwise, as in your example above, size taken on its own is off
> by one (i.e. is rather "last byte" than "size").
> 
> Modern Linux, when logging the memory map, indeed subtracts 1 from
> the sum of addr and size, to show an inclusive range.

We should perhaps do the same then.

If people agree this is the way to go, I can write a patch.

Wei.

> 
> Jan

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel