[Xen-devel] [linux-4.9 test] 113640: regressions - trouble: blocked/broken/fail/pass

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-armhf-libvirt  broken
 build-armhf-pvopsbroken
 build-armhf-libvirt   5 host-build-prep  fail REGR. vs. 113479
 build-armhf-pvops 5 host-build-prep  fail REGR. vs. 113479

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-armhf-armhf-examine  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-vhd   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-credit2   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-cubietruck  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-rtds  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-arndale   1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-xsm   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemut-win7-amd64 18 guest-start/win.repeat fail like 113425
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-localmigrate/x10 fail like 113458
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-localmigrate/x10 fail like 113458
 test-amd64-amd64-xl-rtds 10 debian-install   fail  like 113479
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-qemut-ws16-amd64 10 windows-installfail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qcow2 12 migrate-support-checkfail  never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 13 guest-saverestore   fail never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-installfail never pass
 test-amd64-i386-xl-qemut-ws16-amd64 13 guest-saverestore   fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass
 test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass

version targeted for testing:
 linux089d7720383d7bc9ca6b8824a05dfa66f80d1f41
baseline version:
 linux4ad5dcaca7428dd2bc1a6a40c948e3799c1e27ae

Last test of basis   113479  2017-09-15 15:56:10 Z5 days
Testing same since   113620  2017-09-20 06:49:19 Z1 days2 attempts


People who touched revisions under test:
  Alexei Starovoitov 
  Amir Goldstein 
  Andy Lutomirski 
  Arnd Bergmann 
  Benjamin Poirier 
  Brian Foster 
  Carlos Maiolino 
  Christoph Hellwig 
  Claudiu Manoil 
  Darrick J. Wong 
  David S. Miller 
  Eric Dumazet 
  Eric Sandeen 
  Eric Sandeen 
  Florian Fainelli 
  Florian Westphal 
  Greg Kroah-Hartman 
  Haishuang Yan 
  Ido Schimmel 
  Ingo Molnar 
  Jaegeuk Kim 
  Jan Kara 
  Jason Wang 
  Jesper Dangaard Brouer 
  Jiri Pirko 
  Lukas Czerner 
  Marcelo Ricardo Leitner 
  Martin KaFai Lau 
  Neal Cardwell 
  Nikolay Aleksandrov 
  Nogah Frankel 
  Omar Sandoval 
  Pan Bian 
  Paolo Abeni 
  Paul Menzel 
  Sabrina Dubroca 
  Shaohua Li 
  Song Liu 
  Stefano Brivio 
  Steffen Klassert 
  stephen hemminger 
  Stephen Hemminger 
  Tom Herbert 
  Wei Wang 
  Willem de Bruijn 
  Xin Long 
  Yotam Gigi 
  Yuchung Cheng 

jobs:
 build-amd64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  broken  
 build-i386-libvirt  

Re: [Xen-devel] [PATCH v8 13/15] xen: make grant resource limits per domain

[Juergen Gross]

On 21/09/17 08:15, Jan Beulich wrote:
 On 21.09.17 at 06:35,  wrote:
>> On 20/09/17 17:35, Jan Beulich wrote:
>> On 20.09.17 at 14:44,  wrote:
 On 20/09/17 13:48, Jan Beulich wrote:
 On 20.09.17 at 13:10,  wrote:
>> I thought about a cap and TBH I'm not sure which would be sane to
>> apply. The global limits seem wrong, especially looking at patch 14:
>> those limits will be for dom0 only then. And dom0 won't need many
>> grant frames in the normal case...
>
> I've been thinking about this Dom0 aspect too over lunch. What
> about allowing the hardware domain to set its limit (only upwards
> of course) in setup_table(), without any upper bound enforced?
> This would free up the globals to be used as system wide limits
> again.

 This would be possible, of course.

 The question is whether the need to re-allocate the frame pointer arrays
 is it worth.
>>>
>>> Input by others would be helpful...
>>
>> I think I'll go with additional cap boot parameters, so I don't think
>> we need dom0 to modify its own limits.
> 
> So are we in agreement then that no new command line options
> are needed, and that hence the cap will be applicable to all
> domains (with Dom0 simply not having any other limit enforced
> on it)?

Hmm, I meant this to be the other way round: having distinct parameters
for dom0 and the cap.

In case you like it much better to merge them I won't argue over it.
In this case annotating the variables with __init would be moot, of
course.


Juergen


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

[Xen-devel] [PATCH] xen: support 52 bit physical addresses in pv guests

[Juergen Gross]

Physical addresses on processors supporting 5 level paging can be up to
52 bits wide. For a Xen pv guest running on such a machine those
physical addresses have to be supported in order to be able to use any
memory on the machine even if the guest itself does not support 5 level
paging.

So when reading/writing a MFN from/to a pte don't use the kernel's
PTE_PFN_MASK but a new XEN_PTE_MFN_MASK allowing full 40 bit wide MFNs.

Signed-off-by: Juergen Gross 
---
 arch/x86/include/asm/xen/page.h | 11 ++-
 arch/x86/xen/mmu_pv.c   |  4 ++--
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/xen/page.h b/arch/x86/include/asm/xen/page.h
index 07b6531813c4..bcb8b193c8d1 100644
--- a/arch/x86/include/asm/xen/page.h
+++ b/arch/x86/include/asm/xen/page.h
@@ -26,6 +26,15 @@ typedef struct xpaddr {
phys_addr_t paddr;
 } xpaddr_t;
 
+#ifdef CONFIG_X86_64
+#define XEN_PHYSICAL_MASK  ((1UL << 52) - 1)
+#else
+#define XEN_PHYSICAL_MASK  __PHYSICAL_MASK
+#endif
+
+#define XEN_PTE_MFN_MASK   ((pteval_t)(((signed long)PAGE_MASK) & \
+   XEN_PHYSICAL_MASK))
+
 #define XMADDR(x)  ((xmaddr_t) { .maddr = (x) })
 #define XPADDR(x)  ((xpaddr_t) { .paddr = (x) })
 
@@ -277,7 +286,7 @@ static inline unsigned long bfn_to_local_pfn(unsigned long 
mfn)
 
 static inline unsigned long pte_mfn(pte_t pte)
 {
-   return (pte.pte & PTE_PFN_MASK) >> PAGE_SHIFT;
+   return (pte.pte & XEN_PTE_MFN_MASK) >> PAGE_SHIFT;
 }
 
 static inline pte_t mfn_pte(unsigned long page_nr, pgprot_t pgprot)
diff --git a/arch/x86/xen/mmu_pv.c b/arch/x86/xen/mmu_pv.c
index 509f560bd0c6..958d36d776d9 100644
--- a/arch/x86/xen/mmu_pv.c
+++ b/arch/x86/xen/mmu_pv.c
@@ -315,7 +315,7 @@ void xen_ptep_modify_prot_commit(struct mm_struct *mm, 
unsigned long addr,
 static pteval_t pte_mfn_to_pfn(pteval_t val)
 {
if (val & _PAGE_PRESENT) {
-   unsigned long mfn = (val & PTE_PFN_MASK) >> PAGE_SHIFT;
+   unsigned long mfn = (val & XEN_PTE_MFN_MASK) >> PAGE_SHIFT;
unsigned long pfn = mfn_to_pfn(mfn);
 
pteval_t flags = val & PTE_FLAGS_MASK;
@@ -1740,7 +1740,7 @@ static unsigned long __init m2p(phys_addr_t maddr)
 {
phys_addr_t paddr;
 
-   maddr &= PTE_PFN_MASK;
+   maddr &= XEN_PTE_MFN_MASK;
paddr = mfn_to_pfn(maddr >> PAGE_SHIFT) << PAGE_SHIFT;
 
return paddr;
-- 
2.12.3


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

[Xen-devel] [xen-unstable test] 113638: regressions - FAIL

[osstest service owner]

flight 113638 xen-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/113638/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-credit2  15 guest-saverestorefail REGR. vs. 113387

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-xl-qemuu-debianhvm-amd64 16 guest-localmigrate/x10 fail in 
113618 pass in 113638
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-localmigrate/x10 fail in 113618 
pass in 113638
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 16 guest-localmigrate/x10 fail 
in 113618 pass in 113638
 test-armhf-armhf-xl-credit2 16 guest-start/debian.repeat fail in 113618 pass 
in 113638
 test-armhf-armhf-xl-rtds  6 xen-installfail pass in 113618
 test-amd64-i386-xl-qemuu-win7-amd64 18 guest-start/win.repeat fail pass in 
113618

Regressions which are regarded as allowable (not blocking):
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stopfail REGR. vs. 113387

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeat fail in 113618 blocked 
in 113387
 test-armhf-armhf-xl-rtds13 migrate-support-check fail in 113618 never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-check fail in 113618 never pass
 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail  like 113387
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 113387
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-localmigrate/x10 fail like 113387
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 113387
 test-amd64-amd64-xl-rtds 10 debian-install   fail  like 113387
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 113387
 test-amd64-amd64-xl-qemut-ws16-amd64 10 windows-installfail never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-installfail never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-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-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 13 guest-saverestore   fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  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-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-xl-qemut-ws16-amd64 13 guest-saverestore   fail 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-libvirt-qcow2 12 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  14 saverestore-support-checkfail   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-armhf-armhf-libvirt-raw 12 migrate-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-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass
 test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass

version targeted for testing:
 xen  64cf3181e4d469a8bd7e7dee8ff2d3bf5b45f4b0
baseline version:
 xen  16b1414de91b5a82a0996c67f6db3af7d7e32873

Last test of basis   113387  2017-09-12 23:20:09 Z8 days
Failing since113430  2017-09-14 01:24:48 Z7 days   14 attempts
Testing same since   113618  2017-09-20 06:10:06 Z1 days2 attempts

-

[Xen-devel] [ovmf baseline-only test] 72134: all pass

[Platform Team regression test user]

This run is configured for baseline tests only.

flight 72134 ovmf real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/72134/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 947f3737abf65fda63f3ffd97fddfa6986986868
baseline version:
 ovmf b68c793144e8f239cf59fcc34ee6e35c1fdcd8a6

Last test of basis72132  2017-09-20 21:47:10 Z0 days
Testing same since72134  2017-09-21 06:22:21 Z0 days1 attempts


People who touched revisions under test:
  Aleksei Kovura 
  Laszlo Ersek 

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 pass
 test-amd64-i386-xl-qemuu-ovmf-amd64  pass



sg-report-flight on osstest.xs.citrite.net
logs: /home/osstest/logs
images: /home/osstest/images

Logs, config files, etc. are available at
http://osstest.xs.citrite.net/~osstest/testlogs/logs

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


Push not applicable.


commit 947f3737abf65fda63f3ffd97fddfa6986986868
Author: Laszlo Ersek 
Date:   Tue Sep 19 17:05:08 2017 +0200

OvmfPkg/QemuVideoDxe/VbeShim: handle PAM1 register on Q35 correctly

In commit db27e9f3d8f0 ("OvmfPkg/LegacyRegion: Support legacy region
manipulation of Q35", 2016-03-15), Ray extended the
OvmfPkg/Csm/CsmSupportLib PAM register manipulation to Q35. However, we
missed that the same should be done to the QemuVideoDxe VBE Shim as well.

The omission has caused no problems in practice on Q35, because QEMU has
let us write to the ROM area, regardless of the PAM1 setting, all this
time. This has now changed with recent QEMU commit 208fa0e43645 ("pc: make
'pc.rom' readonly when machine has PCI enabled", 2017-07-28). The QEMU
commit exposes the OVMF bug when Windows 7 is started on Q35, using QEMU
2.10 -- the VBE Shim is no longer put in place and Windows 7 cannot find
it.

To remedy this, assign the "Pam1Address" local variable a PciLib address
that matches the board type (i440fx vs. q35).

Regarding the PcdLib dependency: QemuVideoDxe already uses PcdLib, both
directly (see "PcdDriverSupportedEfiVersion") and indirectly (e.g. via the
DxePciLibI440FxQ35 PciLib instance). Add PcdLib to [LibraryClasses] for
completeness.

Cc: Aleksei Kovura 
Cc: Gerd Hoffmann 
Cc: Igor Mammedov 
Cc: Jordan Justen 
Cc: Ruiyu Ni 
Ref: https://bugs.launchpad.net/qemu/+bug/1715700
Reported-by: Aleksei Kovura 
Special-thanks-to: Gerd Hoffmann 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
Tested-by: Aleksei Kovura 
Reviewed-by: Jordan Justen 

commit ce461ae240a72ff6b0d8a407f004c5db354d91ae
Author: Laszlo Ersek 
Date:   Tue Sep 19 16:48:36 2017 +0200

OvmfPkg/QemuVideoDxe/VbeShim: rename Status to Segment0AllocationStatus

This clarifies the purpose of the local variable in InstallVbeShim().

Cc: Aleksei Kovura 
Cc: Gerd Hoffmann 
Cc: Igor Mammedov 
Cc: Jordan Justen 
Cc: Ruiyu Ni 
Ref: https://bugs.launchpad.net/qemu/+bug/1715700
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
Tested-by: Aleksei Kovura 
Reviewed-by: Jordan Justen 

commit ba1d245f1d3d40c7d87db57dae76e19cbf289718
Author: Laszlo Ersek 
Date:   Tue Sep 19 15:50:39 2017 +0200

OvmfPkg/CsmSupportLib: move PAM register addresses to IndustryStandard

* Introduce the PIIX4_PAM* and MCH_PAM* macros under
  "OvmfPkg/Include/IndustryStandard". These macros capture the PAM
  register offsets (in PCI config space) on the respective Memory
  Controller B/D/F, from the respective data sheets.

* Under IndustryStandard, introduce the PMC_REGISTER_PIIX4() macro for
  PIIX4. (For Q35, we already have DRAMC_REGISTER_Q35().) In both cases,
  the B/D/F is 0/0/0.

* Under CsmSupportLib, replace the "PAMRegOffset" field (UINT8) in the
  PAM_REGISTER_VALUE structure with "PAMRegPciLibAddress" (UINTN). The new
  field contains the return value of the PCI_LIB_ADDRESS() macro.

* Under C

[Xen-devel] [distros-debian-wheezy test] 72135: tolerable trouble: broken/pass

[Platform Team regression test user]

flight 72135 distros-debian-wheezy real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/72135/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 build-arm64   2 hosts-allocate   broken like 72105
 build-arm64-pvops 2 hosts-allocate   broken like 72105
 build-arm64-pvops 3 capture-logs broken like 72105
 build-arm64   3 capture-logs broken like 72105

baseline version:
 flight   72105

jobs:
 build-amd64  pass
 build-arm64  broken  
 build-armhf  pass
 build-i386   pass
 build-amd64-pvopspass
 build-arm64-pvopsbroken  
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-amd64-wheezy-netboot-pvgrub pass
 test-amd64-i386-i386-wheezy-netboot-pvgrub   pass
 test-amd64-i386-amd64-wheezy-netboot-pygrub  pass
 test-amd64-amd64-i386-wheezy-netboot-pygrub  pass



sg-report-flight on osstest.xs.citrite.net
logs: /home/osstest/logs
images: /home/osstest/images

Logs, config files, etc. are available at
http://osstest.xs.citrite.net/~osstest/testlogs/logs

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


Push not applicable.


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

[Xen-devel] [PATCH v1] x86: fix bug caused by commit 0ade5e

[Yi Sun]

Commit 0ade5e causes a bug that only the psr features presented in cmdline
cannot be correctly enumerated.
1. If there is only 'psr=', the CMT is enumerated which is not right.
2. If cmdline is 'psr=cmt,cat,cdp,mba', only the last feature is enumerated.

This patch fixes the issues.

Signed-off-by: Yi Sun 
---
 xen/arch/x86/psr.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index 4515100..daa2aeb 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -422,9 +422,10 @@ static bool __init parse_psr_bool(const char *s, const 
char *delim,
   const char *ss, const char *feature,
   unsigned int mask)
 {
-if ( !strncmp(s, feature, delim - s) )
+/* If cmdline is 'psr=', we need make sure delim != s */
+if ( delim != s && !strncmp(s, feature, delim - s) )
 {
-if ( !*delim )
+if ( !*delim || *delim == ',' )
 opt_psr |= mask;
 else
 {
@@ -457,6 +458,10 @@ static int __init parse_psr_param(const char *s)
 if ( !val_delim )
 val_delim = strchr(s, '\0');
 
+/* E.g. 'psr=cmt,rmid_max:200' */
+if ( val_delim > ss )
+val_delim = ss;
+
 if ( *val_delim && !strncmp(s, "rmid_max", val_delim - s) )
 {
 opt_rmid_max = simple_strtoul(val_delim + 1, &q, 0);
-- 
1.9.1


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

Re: [Xen-devel] [PATCH qemu-traditional] switch to the new ioreq server API

[Vitaly Kuznetsov]

Paul Durrant  writes:

>> -Original Message-
>> From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com]
>> Sent: 06 September 2017 10:29
>> To: xen-devel@lists.xen.org
>> Cc: Paul Durrant ; Ian Jackson
>> 
>> Subject: [PATCH qemu-traditional] switch to the new ioreq server API
>> 
>> Instead of using implicit ioreq server creation side-effect upon reading
>> HVM params switch qemu-traditional to using explicit APIs. This opens a
>> possibility for removing the above mentioned side-effect and special
>> 'default_ioreq_server' code pathes in Xen hypervisor in the future.
>> 
>> This also solves an issue with soft reset when qemu-traditional is being
>> used. Xen c/s e7dabe5 ("x86/hvm: don't unconditionally create a default
>> ioreq server") disabled ioreq server creation after domain was created
>> for the first time and this is needed for soft reset.
>> 
>> IOREQ_TYPE_PCI_CONFIG handling code is stolen as-is from qemu-
>> upstream.
>> 
>> Signed-off-by: Vitaly Kuznetsov 
>
> Reviewed-by: Paul Durrant 
>

Ian,

could you please have a look?

Thanks!

-- 
  Vitaly

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

Re: [Xen-devel] [PATCH v3] xen: credit2: fix spinlock irq-safety violation

[Roger Pau Monné]

On Thu, Sep 21, 2017 at 02:30:37AM +0200, Dario Faggioli wrote:
> In commit ad4b3e1e9df34 ("xen: credit2: implement
> utilization cap") xfree() was being called (for
> deallocating the budget replenishment timer, during
> domain destruction) inside an IRQ disabled critical
> section.
> 
> That must not happen, as it uses the mem-pool's lock,
> which needs to be taken with IRQ enabled. And, in fact,
> we crash (in debug builds):
> 
> (XEN) 
> (XEN) Panic on CPU 0:
> (XEN) Xen BUG at spinlock.c:47
> (XEN) 
> 
> Let's, therefore, kill and deallocate the timer outside of
> the critical sections, when IRQs are enabled.
> 
> Signed-off-by: Dario Faggioli 

Reviewed-by: Roger Pau Monné 

Thanks, Roger.

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

Re: [Xen-devel] pci-passthrough loses msi-x interrupts ability after domain destroy

[Roger Pau Monné]

On Wed, Sep 20, 2017 at 03:50:35PM -0400, Jérôme Oufella wrote:
> Hi Xen-devel, 
> 
> I'm using PCI pass-through to map a PCIe (intel i210) controller into 
> a HVM domain. The system uses xen-pciback to hide the appropriate PCI 
> device from Dom0. 
> 
> When creating the HVM domain after an hypervisor cold boot, the HVM 
> domain can access and use the PCIe controller without problem. 
> 
> However, if the HVM domain is destroyed then restarted, it won't be 
> able to use the pass-through PCI device anymore. The PCI device is 
> seen and can be mapped, however, the interrupts will not be passed to 
> the HVM domain anymore (this is visible under a Linux guest as 
> /proc/interrupts counters remain 0). The behavior on a Windows10 guest 
> is the same. 
> 
> A few interesting hints I noticed: 
> 
> - On Dom0, 'lspci -vv' on that PCIe device between the "working" and 
> the "muted interrupts" states, I noted a difference between the 
> MSI-X caps: 
> 
> - Capabilities: [70] MSI-X: Enable- Count=5 Masked- <-- IRQs will work if 
> domain started 
> + Capabilities: [70] MSI-X: Enable- Count=5 Masked+ <-- IRQs won't work if 
> domain started
> ^^^

IMHO it seems that either your device is not able to perform a reset
successfully, or Linux is not correctly performing such reset. I don't
think there's a lot that can be done from the Xen side.

Thanks, Roger.

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

Re: [Xen-devel] [PATCH v1] x86: fix bug caused by commit 0ade5e

[Juergen Gross]

On 21/09/17 10:11, Yi Sun wrote:
> Commit 0ade5e causes a bug that only the psr features presented in cmdline
> cannot be correctly enumerated.
> 1. If there is only 'psr=', the CMT is enumerated which is not right.
> 2. If cmdline is 'psr=cmt,cat,cdp,mba', only the last feature is enumerated.
> 
> This patch fixes the issues.
> 
> Signed-off-by: Yi Sun 

Reviewed-by: Juergen Gross 


Juergen

> ---
>  xen/arch/x86/psr.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
> index 4515100..daa2aeb 100644
> --- a/xen/arch/x86/psr.c
> +++ b/xen/arch/x86/psr.c
> @@ -422,9 +422,10 @@ static bool __init parse_psr_bool(const char *s, const 
> char *delim,
>const char *ss, const char *feature,
>unsigned int mask)
>  {
> -if ( !strncmp(s, feature, delim - s) )
> +/* If cmdline is 'psr=', we need make sure delim != s */
> +if ( delim != s && !strncmp(s, feature, delim - s) )
>  {
> -if ( !*delim )
> +if ( !*delim || *delim == ',' )
>  opt_psr |= mask;
>  else
>  {
> @@ -457,6 +458,10 @@ static int __init parse_psr_param(const char *s)
>  if ( !val_delim )
>  val_delim = strchr(s, '\0');
>  
> +/* E.g. 'psr=cmt,rmid_max:200' */
> +if ( val_delim > ss )
> +val_delim = ss;
> +
>  if ( *val_delim && !strncmp(s, "rmid_max", val_delim - s) )
>  {
>  opt_rmid_max = simple_strtoul(val_delim + 1, &q, 0);
> 


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

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-freebsd10-amd64 22 leak-check/check  fail REGR. vs. 113626

Regressions which are regarded as allowable (not blocking):
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stopfail REGR. vs. 113626

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 113626
 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail  like 113626
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 113626
 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeatfail  like 113626
 test-amd64-amd64-xl-rtds 10 debian-install   fail  like 113626
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 113626
 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-installfail never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-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-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-qcow2 12 migrate-support-checkfail  never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail 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-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 13 guest-saverestore   fail never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   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-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-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-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass

version targeted for testing:
 qemuub62b7ed0fc9c58e373b8946c9bd2e193be98dae6
baseline version:
 qemuuc51700273ad9802a21c19f8d2b4bcb67c38e74ac

Last test of basis   113626  2017-09-20 09:46:28 Z0 days
Testing same since   113646  2017-09-20 22:32:52 Z0 days1 attempts


People who touched revisions under test:
  Dou Liyang 
  Eduardo Habkost 
  Greg Kurz 
  Igor Mammedov 
  Jan Dakinevich 
  Peter Maydell 
  Philippe Mathieu-Daudé 
  Zack Cornelius 

jobs:
 build-amd64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl 

Re: [Xen-devel] [PATCH v3] xen: credit2: fix spinlock irq-safety violation

[George Dunlap]

On 09/21/2017 01:30 AM, Dario Faggioli wrote:
> In commit ad4b3e1e9df34 ("xen: credit2: implement
> utilization cap") xfree() was being called (for
> deallocating the budget replenishment timer, during
> domain destruction) inside an IRQ disabled critical
> section.
> 
> That must not happen, as it uses the mem-pool's lock,
> which needs to be taken with IRQ enabled. And, in fact,
> we crash (in debug builds):
> 
> (XEN) 
> (XEN) Panic on CPU 0:
> (XEN) Xen BUG at spinlock.c:47
> (XEN) 
> 
> Let's, therefore, kill and deallocate the timer outside of
> the critical sections, when IRQs are enabled.
> 
> Signed-off-by: Dario Faggioli 

Reviewed-by: George Dunlap 

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

Re: [Xen-devel] [PATCH v12 1/4] x86emul: New return code for unimplemented instruction

[Paul Durrant]

> -Original Message-
> From: Petre Pircalabu [mailto:ppircal...@bitdefender.com]
> Sent: 21 September 2017 06:12
> To: xen-devel@lists.xen.org
> Cc: Ian Jackson ; Wei Liu ;
> Andrew Cooper ; George Dunlap
> ; jbeul...@suse.com; konrad.w...@oracle.com;
> sstabell...@kernel.org; Tim (Xen.org) ; Paul Durrant
> ; rcojoc...@bitdefender.com;
> ta...@tklengyel.com; jun.nakaj...@intel.com; Kevin Tian
> ; Petre Pircalabu 
> Subject: [PATCH v12 1/4] x86emul: New return code for unimplemented
> instruction
> 
> Enforce the distinction between an instruction not implemented by the
> emulator and the failure to emulate that instruction by defining a new
> return code, X86EMUL_UNIMPLEMENTED.
> 
> This value should only be returned by the core emulator only if it fails to
> properly decode the current instruction's opcode, and not by any of other
> functions, such as the x86_emulate_ops or the hvm_io_ops callbacks.
> 
> e.g. hvm_process_io_intercept should not return
> X86EMUL_UNIMPLEMENTED.
> The return value of this function depends on either the return code of
> one of the hvm_io_ops handlers (read/write) or the value returned by
> hvm_copy_guest_from_phys / hvm_copy_to_guest_phys.
> 
> Similary, none of this functions should return X86EMUL_UNIMPLEMENTED.
>  - hvm_io_intercept
>  - hvmemul_do_io
>  - hvm_send_buffered_ioreq
>  - hvm_send_ioreq
>  - hvm_broadcast_ioreq
>  - hvmemul_do_io_buffer
>  - hvmemul_validate
> 
> Signed-off-by: Petre Pircalabu 
> 
> ---
> Changed since v10:
> * Added asserts to make sure the return code cannot be
> X86EMUL_UNIMPLEMENTED.
> * Add new return code (X86EMUL_UNRECOGNIZED) to be used when
> trying
> to emulate an instruction with an invalid opcode.
> 
> Changed since v11:
> * Fixed double negative in the patch description.
> * Move assertion into the switch and use ASSERT_UNREACHABLE() when
> applicable.
> * Changed the description of X86EMUL_UNIMPLEMENTED /
> X86EMUL_UNRECOGNIZED
> to reflect the differences between those 2 return codes.
> * Changed the returned value to X86EMUL_UNRECOGNIZED in the
> following cases:
> X86EMUL_OPC(0x0f, 0x73): /* Group 14 */
> X86EMUL_OPC_66(0x0f, 0x73):
> X86EMUL_OPC_VEX_66(0x0f, 0x73):
> - All valid opcodes are defined, so it should return
> X86EMUL_UNRECOGNIZED if mod R/M bits are not matched.
> 
> X86EMUL_OPC(0x0f, 0xc7) /* Group 9 */
> - For register type instructions all possible opcodes are
> defined, so it should return X86EMUL_UNRECOGNIZED if
> mod R/M bits are not matched.
> 
> X86EMUL_OPC_VEX(0x0f38, 0xf3): /* Group 17 */
> - All valid opcodes are defined, so it should return
> X86EMUL_UNRECOGNIZED if mod R/M bits are not matched.
> 
> X86EMUL_OPC_XOP(09, 0x01): /* XOP Grp1 */
> X86EMUL_OPC_XOP(09, 0x02): /* XOP Grp2 */
> - All valid opcodes are defined, so it should return
> X86EMUL_UNRECOGNIZED if mod R/M bits are not matched.
> 
> X86EMUL_OPC(0x0f, 0x01): /* Grp7 */
> - Not all valid opcodes are defined so it should return
> X86EMUL_UNIMPLEMENTED if mod R/M bits are not matched.
> (e.g. XGETBV)
> 
> X86EMUL_OPC_66(0x0f, 0x78):
> - All valid opcodes are defined, so it should return
> X86EMUL_UNRECOGNIZED if mod R/M bits are not matched.
> 
> X86EMUL_OPC(0x0f, 0xae):
> X86EMUL_OPC_66(0x0f, 0xae): /* Grp15 */
> - Not all valid opcodes are defined so it should return
> X86EMUL_UNIMPLEMENTED if mod R/M bits are not matched.
> (e.g. FXSAVE/FXRSTOR )
> ---
>  xen/arch/x86/hvm/emulate.c | 12 
>  xen/arch/x86/hvm/hvm.c |  1 +
>  xen/arch/x86/hvm/io.c  |  1 +
>  xen/arch/x86/hvm/vmx/realmode.c|  2 +-
>  xen/arch/x86/mm/shadow/multi.c |  2 +-
>  xen/arch/x86/x86_emulate/x86_emulate.c | 52 --
> 
>  xen/arch/x86/x86_emulate/x86_emulate.h | 13 +
>  7 files changed, 60 insertions(+), 23 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
> index cc874ce..385fe1e 100644
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -284,10 +284,15 @@ static int hvmemul_do_io(
>  }
>  break;
>  }
> +case X86EMUL_UNIMPLEMENTED:
> +ASSERT_UNREACHABLE();
> +/* Fall-through */

Kind of surprised you need the fall-through if you assert the code is 
unreachable... but analysis tools can be a bit temperamental so ok.

>  default:
>  BUG();
>  }
> 
> +ASSERT(rc != X86EMUL_UNIMPLEMENTED);
> +

Isn't this assertion redundant given the ASSERT_UNREACHABLE() above?

  Paul

>  if ( rc != X86EMUL_OKAY )
>  return rc;
> 
> @@ -313,6 +318,9 @@ s

Re: [Xen-devel] [RFC PATCH V3 1/3] Xen: Increase hap/shadow page pool size to support more vcpus support

[Lan Tianyu]

On 2017年09月20日 23:13, Wei Liu wrote:
> On Tue, Sep 19, 2017 at 11:06:26AM +0800, Lan Tianyu wrote:
>> Hi Wei:
>>
>> On 2017年09月18日 21:06, Wei Liu wrote:
>>> On Wed, Sep 13, 2017 at 12:52:47AM -0400, Lan Tianyu wrote:
 This patch is to increase page pool size when max vcpu number is larger
 than 128.

 Signed-off-by: Lan Tianyu 
 ---
  xen/arch/arm/domain.c|  5 +
  xen/arch/x86/domain.c| 25 +
  xen/common/domctl.c  |  3 +++
  xen/include/xen/domain.h |  2 ++
  4 files changed, 35 insertions(+)

 diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
 index 6512f01..94cf70b 100644
 --- a/xen/arch/arm/domain.c
 +++ b/xen/arch/arm/domain.c
 @@ -824,6 +824,11 @@ int arch_vcpu_reset(struct vcpu *v)
  return 0;
  }
  
 +int arch_domain_set_max_vcpus(struct domain *d)
 +{
 +return 0;
 +}
 +
  static int relinquish_memory(struct domain *d, struct page_list_head 
 *list)
  {
  struct page_info *page, *tmp;
 diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
 index dbddc53..0e230f9 100644
 --- a/xen/arch/x86/domain.c
 +++ b/xen/arch/x86/domain.c
 @@ -1161,6 +1161,31 @@ int arch_vcpu_reset(struct vcpu *v)
  return 0;
  }
  
 +int arch_domain_set_max_vcpus(struct domain *d)
>>>
>>> The name doesn't match what the function does.
>>>
>>
>> I originally hoped to introduce a hook for each arch when set max vcpus.
>> Each arch function can do customized thing and so named
>> "arch_domain_set_max_vcpus".
>>
>> How about "arch_domain_setup_vcpus_resource"?
> 
> Before you go away and do a lot of work, please let us think about if
> this is the right approach first.

Sure. This idea that increase page pool when set max vcpu is from Jan.
Jan, Could you help to check whether current patch is right approach?
Thanks.

> 
> We are close to freeze, with the amount of patches we receive everyday
> RFC patch like this one is low on my (can't speak for others) priority
> list. I am not sure when I will be able to get back to this, but do ping
> us if you want to know where things stand.
> 


-- 
Best regards
Tianyu Lan

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

Re: [Xen-devel] [PATCH v3] xen: credit2: fix spinlock irq-safety violation

[Wei Liu]

On Thu, Sep 21, 2017 at 02:30:37AM +0200, Dario Faggioli wrote:
> In commit ad4b3e1e9df34 ("xen: credit2: implement
> utilization cap") xfree() was being called (for
> deallocating the budget replenishment timer, during
> domain destruction) inside an IRQ disabled critical
> section.
> 
> That must not happen, as it uses the mem-pool's lock,
> which needs to be taken with IRQ enabled. And, in fact,
> we crash (in debug builds):
> 
> (XEN) 
> (XEN) Panic on CPU 0:
> (XEN) Xen BUG at spinlock.c:47
> (XEN) 
> 
> Let's, therefore, kill and deallocate the timer outside of
> the critical sections, when IRQs are enabled.
> 
> Signed-off-by: Dario Faggioli 
> ---

Reviewed-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v4 06/11] mkhex: Move it to tools/misc

[Wei Liu]

On Wed, Sep 20, 2017 at 06:31:43PM -0400, Konrad Rzeszutek Wilk wrote:
> It makes more sense to put a tool to be used by other subsystems
> to be in 'tools/misc' along 'mkrpm','mkdeb', etc.
> 
> The patch titled "xen/livepatch/x86/arm32: Force .livepatch.depends
> section to be uint32_t aligned" uses mkhex.
> 
> Suggested-by: Julien Grall 
> Signed-off-by: Konrad Rzeszutek Wilk 

Acked-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v12 4/4] x86emul: Raise #UD when emulating an unrecognized instruction.

[Paul Durrant]

> -Original Message-
> From: Petre Pircalabu [mailto:ppircal...@bitdefender.com]
> Sent: 21 September 2017 06:12
> To: xen-devel@lists.xen.org
> Cc: Ian Jackson ; Wei Liu ;
> Andrew Cooper ; George Dunlap
> ; jbeul...@suse.com; konrad.w...@oracle.com;
> sstabell...@kernel.org; Tim (Xen.org) ; Paul Durrant
> ; rcojoc...@bitdefender.com;
> ta...@tklengyel.com; jun.nakaj...@intel.com; Kevin Tian
> ; Petre Pircalabu 
> Subject: [PATCH v12 4/4] x86emul: Raise #UD when emulating an
> unrecognized instruction.
> 
> Modified the behavior of hvm_emulate_one_insn and
> vmx_realmode_emulate_one to generate an Invalid Opcode trap when
> X86EMUL_UNRECOGNIZED is returned by the emulator instead of just
> crashing the domain.
> 
> Signed-off-by: Petre Pircalabu 
> Reviewed-by: Kevin Tian 
> ---
>  xen/arch/x86/hvm/io.c   |  6 +-
>  xen/arch/x86/hvm/vmx/realmode.c | 11 ++-
>  2 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
> index 7152c28..c7b1c53 100644
> --- a/xen/arch/x86/hvm/io.c
> +++ b/xen/arch/x86/hvm/io.c
> @@ -96,10 +96,14 @@ bool
> hvm_emulate_one_insn(hvm_emulate_validate_t *validate, const char
> *descr)
>  switch ( rc )
>  {
>  case X86EMUL_UNHANDLEABLE:
> -case X86EMUL_UNIMPLEMENTED:
>  hvm_dump_emulation_state(XENLOG_G_WARNING, descr, &ctxt, rc);
>  return false;
> 
> +case X86EMUL_UNRECOGNIZED:
> +hvm_dump_emulation_state(XENLOG_G_WARNING, descr, &ctxt, rc);
> +hvm_inject_hw_exception(TRAP_invalid_op, X86_EVENT_NO_EC);
> +break;
> +
>  case X86EMUL_EXCEPTION:
>  hvm_inject_event(&ctxt.ctxt.event);
>  break;
> diff --git a/xen/arch/x86/hvm/vmx/realmode.c
> b/xen/arch/x86/hvm/vmx/realmode.c
> index b93792d..03dea6c 100644
> --- a/xen/arch/x86/hvm/vmx/realmode.c
> +++ b/xen/arch/x86/hvm/vmx/realmode.c
> @@ -106,12 +106,21 @@ void vmx_realmode_emulate_one(struct
> hvm_emulate_ctxt *hvmemul_ctxt)
>  if ( hvm_vcpu_io_need_completion(vio) || vio->mmio_retry )
>  vio->io_completion = HVMIO_realmode_completion;
> 
> -if ( rc == X86EMUL_UNHANDLEABLE || rc == X86EMUL_UNIMPLEMENTED
> )
> +if ( rc == X86EMUL_UNHANDLEABLE )

I don't quite understand this change. Why has it become unnecessary to deal 
with X86EMUL_UNIMPLEMENTED? Patch #1 added this change so it seems odd that 
patch #4 would then revert it.

  Paul

>  {
>  gdprintk(XENLOG_ERR, "Failed to emulate insn.\n");
>  goto fail;
>  }
> 
> +if ( rc == X86EMUL_UNRECOGNIZED )
> +{
> +gdprintk(XENLOG_ERR, "Unrecognized insn.\n");
> +if ( curr->arch.hvm_vcpu.guest_cr[0] & X86_CR0_PE )
> +goto fail;
> +
> +realmode_deliver_exception(TRAP_invalid_op, 0, hvmemul_ctxt);
> +}
> +
>  if ( rc == X86EMUL_EXCEPTION )
>  {
>  if ( unlikely(curr->domain->debugger_attached) &&
> --
> 2.7.4


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

[Xen-devel] [ovmf test] 113654: regressions - trouble: blocked/broken/fail/pass

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64-pvopsbroken
 build-amd64-pvops 4 host-install(4)broken REGR. vs. 113647
 build-amd64-xsm   6 xen-buildfail REGR. vs. 113647
 build-i386-xsm6 xen-buildfail REGR. vs. 113647
 build-i3866 xen-buildfail REGR. vs. 113647
 build-amd64   6 xen-buildfail REGR. vs. 113647

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a

version targeted for testing:
 ovmf 9fdf31789a7088736bc574f6802f4a97b5ef2e97
baseline version:
 ovmf 947f3737abf65fda63f3ffd97fddfa6986986868

Last test of basis   113647  2017-09-20 22:34:05 Z0 days
Testing same since   113654  2017-09-21 06:22:39 Z0 days1 attempts


People who touched revisions under test:
  Hao Wu 
  Jian J Wang 

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



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

broken-job build-amd64-pvops broken
broken-step build-amd64-pvops host-install(4)

Not pushing.


commit 9fdf31789a7088736bc574f6802f4a97b5ef2e97
Author: Hao Wu 
Date:   Tue Sep 19 11:01:56 2017 +0800

MdePkg/BaseLib: Avoid reading content beyond string boundary

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=705

As mentioned in the above Bugzilla link by Steven, within the function
PathCleanUpDirectories(), when executing command:
"cd ."

under Shell, the input parameter 'Path' string will have string length
less than 2. Hence, it is possible for the below statement:
"if (StrCmp (Path + StrLen (Path) - 2, L"\\.") == 0) {"

to read contents before the string boundary.

This commit adds additional checks to avoid this.

Cc: Steven Shi 
Cc: Michael Kinney 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
Reviewed-by: Ruiyu Ni 

commit 8c3e4688e0d8e6c218a98855d98976ce46dbb29e
Author: Hao Wu 
Date:   Tue Sep 19 10:22:21 2017 +0800

ShellPkg/Shell: Avoid reading content beyond string boundary

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=690

Within function EfiShellGetDevicePathFromFilePath(), when the input
parameter 'Path' string is like:
"FS0:"

It is possible for the below statement:
"if (*(Path+StrLen(MapName)+1) == CHAR_NULL) {"

to read the content 1 byte beyond the string boundary (both 'Path' and
'MapName' will be FS0: in this case).

This commit adds additional checks to avoid this.

Cc: Steven Shi 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
Reviewed-by: Jaben Carsey 
Reviewed-by: Ruiyu Ni 

commit 14dde9e903bb9a719ebb8f3381da72b19509bc36
Author: Jian J Wang 
Date:   Tue Sep 19 13:52:11 2017 +0800

MdeModulePkg/Core: Fix out-of-sync issue in GCD

From GCD perspective, its SetMemorySpaceAttributes() method doesn't accept 
page
related attributes. That means users cannot use it to change page 
attributes,
and have to turn to CPU arch protocol to do it, which is not be allowe

Re: [Xen-devel] [PULL 0/2] xen-20170920-tag

[Peter Maydell]

On 21 September 2017 at 03:21, Stefano Stabellini
 wrote:
> The following changes since commit b62b7ed0fc9c58e373b8946c9bd2e193be98dae6:
>
>   Merge remote-tracking branch 'remotes/gkurz/tags/for-upstream' into staging 
> (2017-09-20 20:33:48 +0100)
>
> are available in the git repository at:
>
>
>   git://xenbits.xen.org/people/sstabellini/qemu-dm.git tags/xen-20170920-tag
>
> for you to fetch changes up to a8036336609d2e184fc3543a4c439c0ba7d7f3a2:
>
>   xen/pt: allow QEMU to request MSI unmasking at bind time (2017-09-20 
> 19:05:27 -0700)
>
> 
> Xen 2017/09/20
>
> 
> Olaf Hering (1):
>   xen-disk: use g_new0 to fix build
>
> Roger Pau Monne (1):
>   xen/pt: allow QEMU to request MSI unmasking at bind time
>
>  hw/block/xen_disk.c |  2 +-
>  hw/xen/xen_pt.h |  1 +
>  hw/xen/xen_pt_config_init.c | 20 ++--
>  hw/xen/xen_pt_msi.c | 13 ++---
>  4 files changed, 30 insertions(+), 6 deletions(-)

Applied, thanks.

-- PMM

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

[Xen-devel] [libvirt test] 113652: tolerable all pass - PUSHED

[osstest service owner]

flight 113652 libvirt real [real]
http://logs.test-lab.xenproject.org/osstest/logs/113652/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail  like 113616
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 113616
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 113616
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qcow2 12 migrate-support-checkfail  never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass

version targeted for testing:
 libvirt  eae746b2d75973babf1d8b5095db1c7c53573659
baseline version:
 libvirt  dbd98380b9180afab0a9149a2ba4a58bf06d0e02

Last test of basis   113616  2017-09-20 04:20:57 Z1 days
Testing same since   113652  2017-09-21 04:20:55 Z0 days1 attempts


People who touched revisions under test:
  Daniel P. Berrange 
  Julio Faracco 
  Michal Privoznik 
  ZhiPeng Lu 

jobs:
 build-amd64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass
 test-amd64-amd64-libvirt-xsm pass
 test-armhf-armhf-libvirt-xsm pass
 test-amd64-i386-libvirt-xsm  pass
 test-amd64-amd64-libvirt pass
 test-armhf-armhf-libvirt pass
 test-amd64-i386-libvirt  pass
 test-amd64-amd64-libvirt-pairpass
 test-amd64-i386-libvirt-pair pass
 test-amd64-i386-libvirt-qcow2pass
 test-armhf-armhf-libvirt-raw pass
 test-amd64-amd64-libvirt-vhd pass



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


Pushing revision :

+ branch=libvirt
+ revision=eae746b2d75973babf1d8b5095db1c7c53573659
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
 export PERLLIB=.:.
 PERLLIB=.:.
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x '!=' x/home/osstest/repos/lock ']'
++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock
++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push libvirt 
eae746b2d75973babf1d8b5095db1c7c53573659
+ branch=libvirt
+ revision=eae746b2d75973b

Re: [Xen-devel] [PATCH v2 02/21] libxl: introduce a way to mark fields as deprecated in the idl

[Ian Jackson]

Wei Liu writes ("Re: [PATCH v2 02/21] libxl: introduce a way to mark fields as 
deprecated in the idl"):
> On Wed, Sep 20, 2017 at 04:46:16PM +0100, Ian Jackson wrote:
...
> > We discussed how this might be done better.  To me it seems like the
> > only really plausible alternative was to replace the
> > `deprecated_by' and `copy_deprecated_fn' annotations with a single
> > annotation in the parent structure, something like
> >   deprecated_fields=['u.hvm','u',['bootloader_args',
> >   'timer_mode', ...]
> > or maybe even
> >   deprecated_fields=['u.hvm','u',[('timer_mode_new_name','timer_mode')]]
> 
> I know this may sound crazy but: do we need to consider the possibility
> that one field in struct A is deprecated by one field in struct B?

I don't think we can rule that out.  My proposed "deprecated_fields"
thing would be able to cope with that: the information about which
field to copy where is in the struct(s) that contain both A and B.

But, as I say, I don't want to insist on a more comprehensive solution
at this stage.  The IDL compiler language is not a stable interface so
we could choose to do this rework later, when the need arises.

Ian.

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

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

[osstest service owner]

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

Regressions :-(

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

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a

version targeted for testing:
 ovmf 560a435df02b233ea33ae543aeab76b2201de849
baseline version:
 ovmf 947f3737abf65fda63f3ffd97fddfa6986986868

Last test of basis   113647  2017-09-20 22:34:05 Z0 days
Failing since113654  2017-09-21 06:22:39 Z0 days2 attempts
Testing same since   113658  2017-09-21 09:21:11 Z0 days1 attempts


People who touched revisions under test:
  Dandan Bi 
  Hao Wu 
  Jian J Wang 

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



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.


commit 560a435df02b233ea33ae543aeab76b2201de849
Author: Dandan Bi 
Date:   Wed Sep 20 20:19:04 2017 +0800

MdeModulePkg/SetupBrowser: Handle questions with Bit VarStore

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=545

For oneof/numeric/CheckBox(storage can be Bit VarStore)
If the question value can be updated and shown correctly
in UI page, we need do enhancements in following cases:
1. Parse the Ifr data to get the bit VarStore info correctly.
2. Set/get value to/from bit VarStore correctly.

Cc: Eric Dong 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
Reviewed-by: Eric Dong 
Reviewed-by: Liming Gao 

commit 37cd16ac57fcbe5f6ecd15f85ea51621d08cde59
Author: Dandan Bi 
Date:   Wed Sep 20 20:09:04 2017 +0800

MdeModulePkg/HiiDatabase: Handle questions with Bit VarStore

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=545

For oneof/numeric/checkbox, their storage may be bit field.
When generating  string to get default value
for these questions, we need to parse the Ifr data to get
the bit Varstore info,and then generating the correct
 string.

Cc: Eric Dong 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
Reviewed-by: Eric Dong 
Reviewed-by: Liming Gao 

commit 95a71351919aee15874b748f1aae2f8f492d2f76
Author: Dandan Bi 
Date:   Wed Sep 20 19:43:45 2017 +0800

MdeModulePkg/UefiHiiLib: Validate question with bit fields

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=545

In UefiHiiLib, there are codes to validate the current setting of
questions, now update the logic to handle question with bit storage.

Cc: Eric Dong 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
Reviewed-by: Eric Dong 
Reviewed-by: Liming Gao 

commit 01723271a8baef1dc4c92bce0c09d41055cc5eb9
Author: Dandan Bi 
Date:   Wed Sep 20 19:24:18 2017 +0800

MdeModulePkg: Add GUID/flags to implement BitField support

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=545

Re: [Xen-devel] [RFC 6/6] acpi:arm64: Add support for parsing IORT table

[Julien Grall]

Hi Sameer,

On 21/09/17 01:37, Goel, Sameer wrote:


[...]


diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index 995a85a..3785fae 100644
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -9,7 +9,12 @@
   #include 

   #define BUG_ON(p)  do { if (unlikely(p)) BUG();  } while (0)
-#define WARN_ON(p) do { if (unlikely(p)) WARN(); } while (0)
+#define WARN_ON(p) ({  \
+int __ret_warn_on = !!(p); \
+if (unlikely(__ret_warn_on))   \
+WARN();\
+unlikely(__ret_warn_on);   \
+})


h. Why this change?

Was getting a compilation error when I was using WARN_ON as a conditional
in an if statement regarding the return value. So removed the loop. This
looks similar to Linux now.


This should belong to a separate patch with a commit message explaining why the 
change.


Agreed. Since this is more of a Linux compat function, I do not want to change 
the system-wide behavior. I will move this in the IORT code.


I don't see any reason to keep this implementation only for IORT. It 
would be possible to have similar construction in Xen.


So I would first try to see if maintainers would be willing to accept a 
system-wide change before thinking to re-implement WARN_ON in IORT.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v2 2/3] Introduce migration precopy policy

[Roger Pau Monné]

On Wed, Sep 20, 2017 at 05:18:16PM +0100, Jennifer Herbert wrote:
> On 20/09/17 11:20, Roger Pau Monné wrote:
> > On Tue, Sep 19, 2017 at 07:06:26PM +0100, Jennifer Herbert wrote:
> > > +? XGS_POLICY_STOP_AND_COPY
> > > +: XGS_POLICY_CONTINUE_PRECOPY;
> > > +}
> > > +
> > > +/*
> > >* Send memory while guest is running.
> > >*/
> > >   static int send_memory_live(struct xc_sr_context *ctx)
> > > @@ -474,21 +491,58 @@ static int send_memory_live(struct xc_sr_context 
> > > *ctx)
> > >   xc_interface *xch = ctx->xch;
> > >   xc_shadow_op_stats_t stats = { 0, ctx->save.p2m_size };
> > >   char *progress_str = NULL;
> > > -unsigned x;
> > > +unsigned int x = 0;
> > >   int rc;
> > > +int policy_decision;
> > > +
> > > +DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap,
> > > +&ctx->save.dirty_bitmap_hbuf);
> > > +
> > > +precopy_policy_t precopy_policy = 
> > > ctx->save.callbacks->precopy_policy;
> > > +void *data = ctx->save.callbacks->data;
> > > +
> > > +struct precopy_stats *policy_stats;
> > >   rc = update_progress_string(ctx, &progress_str, 0);
> > >   if ( rc )
> > >   goto out;
> > > -rc = send_all_pages(ctx);
> > > -if ( rc )
> > > -goto out;
> > > +ctx->save.stats = (struct precopy_stats)
> > > +{ .dirty_count   = ctx->save.p2m_size };
> > This is exactly the same as 'stats' at this point. I'm slightly
> > confused about why you need 2 different stats variable, plus a pointer
> > to a stats variable (stats, ctx->save.stats and *policy_stats).
> 
> They do start off similar, and are certainly closely related.
> xc_shadow_op_stats_t stats has different fields in it then precopy_stats
> policy_stats.
> The former has a fault and dirty count, per iteration, while the latter has
> iteration number, total_written (over all iterations) and dirty count.

OK. I'm not that familiar with this code, so maybe this doesn't make
sense, but wouldn't it be clearer to expand the xc_shadow_op_stats_t
type so that a single variable can contain all this information?

I find it slightly confusing to use two variables of the same type
that track different things.

> *policy_stats  is just a convenience pointer, reducing the amount of
> indirection on
> every access.  I though this made it easier to read.
> 
> > > +policy_stats = &ctx->save.stats;
> > > +
> > > +if ( precopy_policy == NULL )
> > > + precopy_policy = simple_precopy_policy;
> > > +
> > > +bitmap_set(dirty_bitmap, ctx->save.p2m_size);
> > > +
> > > +do {
> > > +policy_decision = precopy_policy(*policy_stats, data);
> > The comment at the top says:
> > 
> > "Called after every batch of page data sent during the precopy phase"
> > 
> > Yet here the hook seems to be called before any processing has been
> > done for the first iteration of the loop.
> 
> I'll change to "Called before and after every batch "
> 
> > > +x++;
> > Also updating x here seems weird, we completely ignore iteration 0.
> 
> The line above the 'x++' checks the policy using 'iteration 0'.  In
> patch v1 I used the x variable in initialising the stats, to try and
> suggest this, but as its zero, and the default value for a struct is
> zero, it was concluded that was unnecessary.  In any case,
> logically, this is where it moves from one 'iteration' to another.
> Previously there was no iteration zero, as it started on zero.
> Now iteration zero is to indicate the starting state.
> 
> Combining this comment with Paul's, it could use:
> for (x = 1; ; ++x)
> If this is thought to be more readable - although Andrew cooper
> described a loop looking like this as "suspicious" on Joshua's version
> of this patch.
> 
> I have no strong feelings on the matter let me know.

I don't really have a strong opinion, I tend to use 'for ( ; ; )' for
unbounded loops, but it's mostly a question of taste.

Thanks, Roger.

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

Re: [Xen-devel] [PATCH v2 2/3] Introduce migration precopy policy

[Wei Liu]

On Thu, Sep 21, 2017 at 12:08:04PM +0100, Roger Pau Monné wrote:
> On Wed, Sep 20, 2017 at 05:18:16PM +0100, Jennifer Herbert wrote:
> > On 20/09/17 11:20, Roger Pau Monné wrote:
> > > On Tue, Sep 19, 2017 at 07:06:26PM +0100, Jennifer Herbert wrote:
> > > > +? XGS_POLICY_STOP_AND_COPY
> > > > +: XGS_POLICY_CONTINUE_PRECOPY;
> > > > +}
> > > > +
> > > > +/*
> > > >* Send memory while guest is running.
> > > >*/
> > > >   static int send_memory_live(struct xc_sr_context *ctx)
> > > > @@ -474,21 +491,58 @@ static int send_memory_live(struct xc_sr_context 
> > > > *ctx)
> > > >   xc_interface *xch = ctx->xch;
> > > >   xc_shadow_op_stats_t stats = { 0, ctx->save.p2m_size };
> > > >   char *progress_str = NULL;
> > > > -unsigned x;
> > > > +unsigned int x = 0;
> > > >   int rc;
> > > > +int policy_decision;
> > > > +
> > > > +DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap,
> > > > +&ctx->save.dirty_bitmap_hbuf);
> > > > +
> > > > +precopy_policy_t precopy_policy = 
> > > > ctx->save.callbacks->precopy_policy;
> > > > +void *data = ctx->save.callbacks->data;
> > > > +
> > > > +struct precopy_stats *policy_stats;
> > > >   rc = update_progress_string(ctx, &progress_str, 0);
> > > >   if ( rc )
> > > >   goto out;
> > > > -rc = send_all_pages(ctx);
> > > > -if ( rc )
> > > > -goto out;
> > > > +ctx->save.stats = (struct precopy_stats)
> > > > +{ .dirty_count   = ctx->save.p2m_size };
> > > This is exactly the same as 'stats' at this point. I'm slightly
> > > confused about why you need 2 different stats variable, plus a pointer
> > > to a stats variable (stats, ctx->save.stats and *policy_stats).
> > 
> > They do start off similar, and are certainly closely related.
> > xc_shadow_op_stats_t stats has different fields in it then precopy_stats
> > policy_stats.
> > The former has a fault and dirty count, per iteration, while the latter has
> > iteration number, total_written (over all iterations) and dirty count.
> 
> OK. I'm not that familiar with this code, so maybe this doesn't make
> sense, but wouldn't it be clearer to expand the xc_shadow_op_stats_t
> type so that a single variable can contain all this information?
> 
> I find it slightly confusing to use two variables of the same type
> that track different things.
> 

The xc_shadow_op_stats_t is in fact xen_domctl_shadow_op_stats, which
gets passed directly to the hypervisor. So I think having two separate
structs here is okay. They are describing different things after all.

> > *policy_stats  is just a convenience pointer, reducing the amount of
> > indirection on
> > every access.  I though this made it easier to read.
> > 
> > > > +policy_stats = &ctx->save.stats;
> > > > +
> > > > +if ( precopy_policy == NULL )
> > > > + precopy_policy = simple_precopy_policy;
> > > > +
> > > > +bitmap_set(dirty_bitmap, ctx->save.p2m_size);
> > > > +
> > > > +do {
> > > > +policy_decision = precopy_policy(*policy_stats, data);
> > > The comment at the top says:
> > > 
> > > "Called after every batch of page data sent during the precopy phase"
> > > 
> > > Yet here the hook seems to be called before any processing has been
> > > done for the first iteration of the loop.
> > 
> > I'll change to "Called before and after every batch "
> > 
> > > > +x++;
> > > Also updating x here seems weird, we completely ignore iteration 0.
> > 
> > The line above the 'x++' checks the policy using 'iteration 0'.  In
> > patch v1 I used the x variable in initialising the stats, to try and
> > suggest this, but as its zero, and the default value for a struct is
> > zero, it was concluded that was unnecessary.  In any case,
> > logically, this is where it moves from one 'iteration' to another.
> > Previously there was no iteration zero, as it started on zero.
> > Now iteration zero is to indicate the starting state.
> > 
> > Combining this comment with Paul's, it could use:
> > for (x = 1; ; ++x)
> > If this is thought to be more readable - although Andrew cooper
> > described a loop looking like this as "suspicious" on Joshua's version
> > of this patch.
> > 
> > I have no strong feelings on the matter let me know.
> 
> I don't really have a strong opinion, I tend to use 'for ( ; ; )' for
> unbounded loops, but it's mostly a question of taste.
> 

I don't care either. Please pick the style you like. ;-)

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

[Xen-devel] [PATCH v4] xen: grant-table: Simplify get_paged_frame

[Julien Grall]

The implementation of get_paged_frame is currently different whether the
architecture support sharing memory or paging memory. Both
version are extremely similar so it is possible to consolidate in a
single implementation.

The main difference is the x86 version will allow grant on foreign page
when using HVM/PVH whilst Arm does not. At the moment, on x86 foreign pages
are only allowed for PVH Dom0. It seems that foreign pages should never
be granted so deny them

The check for shared/paged memory are now gated with the respective ifdef.
Potentially, dummy p2m_is_shared/p2m_is_paging could be implemented for
Arm.

Lastly remove pointless parenthesis in the code modified.

Signed-off-by: Julien Grall 
Reviewed-by: Wei Liu 

---

Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Konrad Rzeszutek Wilk 
Cc: Stefano Stabellini 
Cc: Tim Deegan 
Cc: Wei Liu 

Changes in v4:
- Update variables on error path to match the comment
- Add Wei's reviewed-by

Changes in v3:
- Add missing put_page in the error path
- Remove pointless parenthesis

Changes in v2:
- Deny grant on foreign page (aligned with the ARM code)
- Use #ifdef rather than #if defined
- Update commit message
- Fix typo in the title

get_page_from_gfn will be able to get reference on foreign page and as
per my understanding will allow to grant page on foreign memory.

This was not allowed with a simple get_page(...) on the ARM
implementation (no sharing nor paging supprot) but is allowed on the x86
implementation due to get_page_from_gfn.

On x86, foreign pages are currently only allowed for PVH dom0, so I
think it is not a big deal for now.

On Arm, foreign pages can be present on any domain. So this patch would
permit grant on foreing pages.

This patch will deny granting foreign pages. Jan Beulich is happy with
it. Any other opinions?
---
 xen/common/grant_table.c | 26 ++
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index f48eeff7ad..0f09891f59 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -338,34 +338,36 @@ static int get_paged_frame(unsigned long gfn, unsigned 
long *frame,
struct domain *rd)
 {
 int rc = GNTST_okay;
-#if defined(P2M_PAGED_TYPES) || defined(P2M_SHARED_TYPES)
 p2m_type_t p2mt;
 
+*frame = mfn_x(INVALID_MFN);
 *page = get_page_from_gfn(rd, gfn, &p2mt,
-  (readonly) ? P2M_ALLOC : P2M_UNSHARE);
-if ( !(*page) )
+  readonly ? P2M_ALLOC : P2M_UNSHARE);
+if ( !*page )
 {
-*frame = mfn_x(INVALID_MFN);
+#ifdef P2M_SHARED_TYPES
 if ( p2m_is_shared(p2mt) )
 return GNTST_eagain;
+#endif
+#ifdef P2M_PAGES_TYPES
 if ( p2m_is_paging(p2mt) )
 {
 p2m_mem_paging_populate(rd, gfn);
 return GNTST_eagain;
 }
+#endif
 return GNTST_bad_page;
 }
-*frame = page_to_mfn(*page);
-#else
-*frame = mfn_x(gfn_to_mfn(rd, _gfn(gfn)));
-*page = mfn_valid(_mfn(*frame)) ? mfn_to_page(*frame) : NULL;
-if ( (!(*page)) || (!get_page(*page, rd)) )
+
+if ( p2m_is_foreign(p2mt) )
 {
-*frame = mfn_x(INVALID_MFN);
+put_page(*page);
 *page = NULL;
-rc = GNTST_bad_page;
+
+return GNTST_bad_page;
 }
-#endif
+
+*frame = page_to_mfn(*page);
 
 return rc;
 }
-- 
2.11.0


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

Re: [Xen-devel] [RFC PATCH V3 1/3] Xen: Increase hap/shadow page pool size to support more vcpus support

[Jan Beulich]

>>> On 21.09.17 at 10:50,  wrote:
> On 2017年09月20日 23:13, Wei Liu wrote:
>> On Tue, Sep 19, 2017 at 11:06:26AM +0800, Lan Tianyu wrote:
>>> Hi Wei:
>>>
>>> On 2017年09月18日 21:06, Wei Liu wrote:
 On Wed, Sep 13, 2017 at 12:52:47AM -0400, Lan Tianyu wrote:
> This patch is to increase page pool size when max vcpu number is larger
> than 128.
>
> Signed-off-by: Lan Tianyu 
> ---
>  xen/arch/arm/domain.c|  5 +
>  xen/arch/x86/domain.c| 25 +
>  xen/common/domctl.c  |  3 +++
>  xen/include/xen/domain.h |  2 ++
>  4 files changed, 35 insertions(+)
>
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 6512f01..94cf70b 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -824,6 +824,11 @@ int arch_vcpu_reset(struct vcpu *v)
>  return 0;
>  }
>  
> +int arch_domain_set_max_vcpus(struct domain *d)
> +{
> +return 0;
> +}
> +
>  static int relinquish_memory(struct domain *d, struct page_list_head 
> *list)
>  {
>  struct page_info *page, *tmp;
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index dbddc53..0e230f9 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -1161,6 +1161,31 @@ int arch_vcpu_reset(struct vcpu *v)
>  return 0;
>  }
>  
> +int arch_domain_set_max_vcpus(struct domain *d)

 The name doesn't match what the function does.

>>>
>>> I originally hoped to introduce a hook for each arch when set max vcpus.
>>> Each arch function can do customized thing and so named
>>> "arch_domain_set_max_vcpus".
>>>
>>> How about "arch_domain_setup_vcpus_resource"?
>> 
>> Before you go away and do a lot of work, please let us think about if
>> this is the right approach first.
> 
> Sure. This idea that increase page pool when set max vcpu is from Jan.
> Jan, Could you help to check whether current patch is right approach?

Whenever I get to it, sure. What I can say right away is that I
agree with the comment about the name of the function.

Jan

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

Re: [Xen-devel] [PATCH v8 13/15] xen: make grant resource limits per domain

[Jan Beulich]

>>> On 21.09.17 at 09:53,  wrote:
> On 21/09/17 08:15, Jan Beulich wrote:
> On 21.09.17 at 06:35,  wrote:
>>> On 20/09/17 17:35, Jan Beulich wrote:
>>> On 20.09.17 at 14:44,  wrote:
> On 20/09/17 13:48, Jan Beulich wrote:
> On 20.09.17 at 13:10,  wrote:
>>> I thought about a cap and TBH I'm not sure which would be sane to
>>> apply. The global limits seem wrong, especially looking at patch 14:
>>> those limits will be for dom0 only then. And dom0 won't need many
>>> grant frames in the normal case...
>>
>> I've been thinking about this Dom0 aspect too over lunch. What
>> about allowing the hardware domain to set its limit (only upwards
>> of course) in setup_table(), without any upper bound enforced?
>> This would free up the globals to be used as system wide limits
>> again.
>
> This would be possible, of course.
>
> The question is whether the need to re-allocate the frame pointer arrays
> is it worth.

 Input by others would be helpful...
>>>
>>> I think I'll go with additional cap boot parameters, so I don't think
>>> we need dom0 to modify its own limits.
>> 
>> So are we in agreement then that no new command line options
>> are needed, and that hence the cap will be applicable to all
>> domains (with Dom0 simply not having any other limit enforced
>> on it)?
> 
> Hmm, I meant this to be the other way round: having distinct parameters
> for dom0 and the cap.
> 
> In case you like it much better to merge them I won't argue over it.

Well, late yesterday evening it occurred to me that it would
only be consistent to apply the same cap to all domains. That's
in particular to not penalize a non-Dom0 hardware domain in
comparison with Dom0 itself.

> In this case annotating the variables with __init would be moot, of
> course.

Of course.

Jan


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

[Xen-devel] [PATCH] xen/perfc: Fix build after commit fc32575968 when CONFIG_PERF_COUNTERS=y

[Julien Grall]

The commit fc32575968 "public/sysctl: drop unnecessary typedefs and
handles" went a bit too far by replacing all xen_systcl_*_t type to
struct xen_sysctl_*.

However, xen_sysctl_perfc_val_t was a typedef on uint32_t and therefore
is not associated to a structure.

Use xen_sysctl_perfc_val_t to fix the build when CONFIG_PERF_COUNTERS=y

Signed-off-by: Julien Grall 
---
 xen/common/perfc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/common/perfc.c b/xen/common/perfc.c
index 76051739a5..067567766a 100644
--- a/xen/common/perfc.c
+++ b/xen/common/perfc.c
@@ -153,7 +153,7 @@ void perfc_reset(unsigned char key)
 }
 
 static struct xen_sysctl_perfc_desc perfc_d[NR_PERFCTRS];
-static struct xen_sysctl_perfc_val *perfc_vals;
+static xen_sysctl_perfc_val_t *perfc_vals;
 static unsigned int  perfc_nbr_vals;
 static cpumask_t perfc_cpumap;
 
@@ -190,7 +190,7 @@ static int 
perfc_copy_info(XEN_GUEST_HANDLE_64(xen_sysctl_perfc_desc_t) desc,
 }
 
 xfree(perfc_vals);
-perfc_vals = xmalloc_array(struct xen_sysctl_perfc_val, 
perfc_nbr_vals);
+perfc_vals = xmalloc_array(xen_sysctl_perfc_val_t, perfc_nbr_vals);
 }
 
 if ( guest_handle_is_null(desc) )
-- 
2.11.0


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

Re: [Xen-devel] [PATCH v8 13/15] xen: make grant resource limits per domain

[Juergen Gross]

On 21/09/17 13:31, Jan Beulich wrote:
 On 21.09.17 at 09:53,  wrote:
>> On 21/09/17 08:15, Jan Beulich wrote:
>> On 21.09.17 at 06:35,  wrote:
 On 20/09/17 17:35, Jan Beulich wrote:
 On 20.09.17 at 14:44,  wrote:
>> On 20/09/17 13:48, Jan Beulich wrote:
>> On 20.09.17 at 13:10,  wrote:
 I thought about a cap and TBH I'm not sure which would be sane to
 apply. The global limits seem wrong, especially looking at patch 14:
 those limits will be for dom0 only then. And dom0 won't need many
 grant frames in the normal case...
>>>
>>> I've been thinking about this Dom0 aspect too over lunch. What
>>> about allowing the hardware domain to set its limit (only upwards
>>> of course) in setup_table(), without any upper bound enforced?
>>> This would free up the globals to be used as system wide limits
>>> again.
>>
>> This would be possible, of course.
>>
>> The question is whether the need to re-allocate the frame pointer arrays
>> is it worth.
>
> Input by others would be helpful...

 I think I'll go with additional cap boot parameters, so I don't think
 we need dom0 to modify its own limits.
>>>
>>> So are we in agreement then that no new command line options
>>> are needed, and that hence the cap will be applicable to all
>>> domains (with Dom0 simply not having any other limit enforced
>>> on it)?
>>
>> Hmm, I meant this to be the other way round: having distinct parameters
>> for dom0 and the cap.
>>
>> In case you like it much better to merge them I won't argue over it.
> 
> Well, late yesterday evening it occurred to me that it would
> only be consistent to apply the same cap to all domains. That's
> in particular to not penalize a non-Dom0 hardware domain in
> comparison with Dom0 itself.

That's correct.

OTOH e.g. a cap of lets say 1024 grant frames but Dom0 configured to
4 only (why would it need more?) would make sense: the grant frame array
for Dom0 would need 32 bytes only instead of the 8kB for the 1024 frames
if the cap would be the configuration value for Dom0.


Juergen

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

Re: [Xen-devel] [PATCH] xen/perfc: Fix build after commit fc32575968 when CONFIG_PERF_COUNTERS=y

[Wei Liu]

On Thu, Sep 21, 2017 at 12:35:17PM +0100, Julien Grall wrote:
> The commit fc32575968 "public/sysctl: drop unnecessary typedefs and
> handles" went a bit too far by replacing all xen_systcl_*_t type to
> struct xen_sysctl_*.
> 
> However, xen_sysctl_perfc_val_t was a typedef on uint32_t and therefore
> is not associated to a structure.
> 
> Use xen_sysctl_perfc_val_t to fix the build when CONFIG_PERF_COUNTERS=y
> 
> Signed-off-by: Julien Grall 

Reviewed-by: Wei Liu 

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

[Xen-devel] About evtchn and netfront/netback changes

[Kun Cheng]

Hello guys,

I'm porting some legacy code initially developed in Xen 4.0 and Linux 2.6
to longterm support kernels 3.2.93 and 4.4.88, and Xen 4.6 or 4.8 (not
decided yet but that's not the key issue). My legacy work involves dealing
with event channel and xen-netfront/netback drivers. However, I found a lot
of things in Linux 3.2.93 and 4.4.88 are different from Linux 2.6.32. I
would be appreciated if anyone knows about detailed API changes (especially
the event channel and pv net driver parts). Currently all googling results
I found are about linux 2.6 and Xen 4.0/3.x, I will keep digging the devel
mailing list.

Thanks,
Kun
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v8 13/15] xen: make grant resource limits per domain

[Jan Beulich]

>>> On 21.09.17 at 13:39,  wrote:
> On 21/09/17 13:31, Jan Beulich wrote:
> On 21.09.17 at 09:53,  wrote:
>>> On 21/09/17 08:15, Jan Beulich wrote:
>>> On 21.09.17 at 06:35,  wrote:
> On 20/09/17 17:35, Jan Beulich wrote:
> On 20.09.17 at 14:44,  wrote:
>>> On 20/09/17 13:48, Jan Beulich wrote:
>>> On 20.09.17 at 13:10,  wrote:
> I thought about a cap and TBH I'm not sure which would be sane to
> apply. The global limits seem wrong, especially looking at patch 14:
> those limits will be for dom0 only then. And dom0 won't need many
> grant frames in the normal case...

 I've been thinking about this Dom0 aspect too over lunch. What
 about allowing the hardware domain to set its limit (only upwards
 of course) in setup_table(), without any upper bound enforced?
 This would free up the globals to be used as system wide limits
 again.
>>>
>>> This would be possible, of course.
>>>
>>> The question is whether the need to re-allocate the frame pointer arrays
>>> is it worth.
>>
>> Input by others would be helpful...
>
> I think I'll go with additional cap boot parameters, so I don't think
> we need dom0 to modify its own limits.

 So are we in agreement then that no new command line options
 are needed, and that hence the cap will be applicable to all
 domains (with Dom0 simply not having any other limit enforced
 on it)?
>>>
>>> Hmm, I meant this to be the other way round: having distinct parameters
>>> for dom0 and the cap.
>>>
>>> In case you like it much better to merge them I won't argue over it.
>> 
>> Well, late yesterday evening it occurred to me that it would
>> only be consistent to apply the same cap to all domains. That's
>> in particular to not penalize a non-Dom0 hardware domain in
>> comparison with Dom0 itself.
> 
> That's correct.
> 
> OTOH e.g. a cap of lets say 1024 grant frames but Dom0 configured to
> 4 only (why would it need more?) would make sense: the grant frame array
> for Dom0 would need 32 bytes only instead of the 8kB for the 1024 frames
> if the cap would be the configuration value for Dom0.

May I suggest that for now we use the simpler variant without
extra Dom0 command line options, and later (post 4.10), if you or
anyone else really feels like it, Dom0 specific options be introduced?

Jan


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

Re: [Xen-devel] [PATCH v8 13/15] xen: make grant resource limits per domain

[Juergen Gross]

On 21/09/17 13:48, Jan Beulich wrote:
 On 21.09.17 at 13:39,  wrote:
>> On 21/09/17 13:31, Jan Beulich wrote:
>> On 21.09.17 at 09:53,  wrote:
 On 21/09/17 08:15, Jan Beulich wrote:
 On 21.09.17 at 06:35,  wrote:
>> On 20/09/17 17:35, Jan Beulich wrote:
>> On 20.09.17 at 14:44,  wrote:
 On 20/09/17 13:48, Jan Beulich wrote:
 On 20.09.17 at 13:10,  wrote:
>> I thought about a cap and TBH I'm not sure which would be sane to
>> apply. The global limits seem wrong, especially looking at patch 14:
>> those limits will be for dom0 only then. And dom0 won't need many
>> grant frames in the normal case...
>
> I've been thinking about this Dom0 aspect too over lunch. What
> about allowing the hardware domain to set its limit (only upwards
> of course) in setup_table(), without any upper bound enforced?
> This would free up the globals to be used as system wide limits
> again.

 This would be possible, of course.

 The question is whether the need to re-allocate the frame pointer 
 arrays
 is it worth.
>>>
>>> Input by others would be helpful...
>>
>> I think I'll go with additional cap boot parameters, so I don't think
>> we need dom0 to modify its own limits.
>
> So are we in agreement then that no new command line options
> are needed, and that hence the cap will be applicable to all
> domains (with Dom0 simply not having any other limit enforced
> on it)?

 Hmm, I meant this to be the other way round: having distinct parameters
 for dom0 and the cap.

 In case you like it much better to merge them I won't argue over it.
>>>
>>> Well, late yesterday evening it occurred to me that it would
>>> only be consistent to apply the same cap to all domains. That's
>>> in particular to not penalize a non-Dom0 hardware domain in
>>> comparison with Dom0 itself.
>>
>> That's correct.
>>
>> OTOH e.g. a cap of lets say 1024 grant frames but Dom0 configured to
>> 4 only (why would it need more?) would make sense: the grant frame array
>> for Dom0 would need 32 bytes only instead of the 8kB for the 1024 frames
>> if the cap would be the configuration value for Dom0.
> 
> May I suggest that for now we use the simpler variant without
> extra Dom0 command line options, and later (post 4.10), if you or
> anyone else really feels like it, Dom0 specific options be introduced?

NP. I just wanted to point it out.


Juergen


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

Re: [Xen-devel] [PATCH 1/3] python: Add binding for xs_fileno()

[Wei Liu]

On Fri, Sep 15, 2017 at 05:35:34PM +0100, Euan Harris wrote:
> xs_fileno() returns a file descriptor which receives events when Xenstore
> watches fire.   Exposing this in the Python bindings is a prerequisite
> for writing event-driven clients in Python.
> 
> Signed-off-by: Euan Harris 

Reviewed-by: Wei Liu 

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

Re: [Xen-devel] [PATCH 2/3] python: Extract registered watch search logic from xspy_read_watch()

[Wei Liu]

On Fri, Sep 15, 2017 at 05:35:35PM +0100, Euan Harris wrote:
>  tools/python/xen/lowlevel/xs/xs.c | 61 
> ---
>  1 file changed, 38 insertions(+), 23 deletions(-)
> 
> diff --git a/tools/python/xen/lowlevel/xs/xs.c 
> b/tools/python/xen/lowlevel/xs/xs.c
> index 9f1b916..a4b50a0 100644
> --- a/tools/python/xen/lowlevel/xs/xs.c
> +++ b/tools/python/xen/lowlevel/xs/xs.c
> @@ -77,6 +77,8 @@ static inline struct xs_handle *xshandle(XsHandle *self)
>  
>  static void remove_watch(XsHandle *xsh, PyObject *token);
>  
> +static PyObject *match_watch_by_token(XsHandle *self, char **xsval);
> +
>  static PyObject *none(bool result);
>  
>  static int parse_transaction_path(XsHandle *self, PyObject *args,
> @@ -484,8 +486,6 @@ static PyObject *xspy_read_watch(XsHandle *self, PyObject 
> *args)
>  struct xs_handle *xh = xshandle(self);
>  PyObject *val = NULL;
>  char **xsval;
> -PyObject *token;
> -int i;
>  unsigned int num;
>  
>  if (!xh)
> @@ -497,32 +497,20 @@ again:
>  Py_END_ALLOW_THREADS
>  if (!xsval) {
>  PyErr_SetFromErrno(xs_error);
> -goto exit;
> -}
> -if (sscanf(xsval[XS_WATCH_TOKEN], "%li", (unsigned long *)&token) != 1) {
> - xs_set_error(EINVAL);
> -goto exit;
> -}
> -for (i = 0; i < PyList_Size(self->watches); i++) {
> -if (token == PyList_GetItem(self->watches, i))
> -break;
> -}
> -if (i == PyList_Size(self->watches)) {
> -  /* We do not have a registered watch for the one that has just fired.
> - Ignore this -- a watch that has been recently deregistered can still
> - have watches in transit.  This is a blocking method, so go back to
> - read again.
> -  */
> -  free(xsval);
> -  goto again;
> +return val;
>  }
> -/* Create tuple (path, token). */
> -val = Py_BuildValue("(sO)", xsval[XS_WATCH_PATH], token);
> - exit:
> +
> +val = match_watch_by_token(self, xsval);
>  free(xsval);
> +
> +if (!val && errno == EAGAIN) {
> +goto again;
> +}
> +
>  return val;
>  }
>  
> +

Stray blank line.

>  #define xspy_unwatch_doc "\n"\
>   "Stop watching a path.\n"   \
>   " path  [string] : xenstore path.\n"\
> @@ -868,6 +856,33 @@ static int parse_transaction_path(XsHandle *self, 
> PyObject *args,
>  }
>  
>  
> +static PyObject *match_watch_by_token(XsHandle *self, char **xsval)
> +{
> +PyObject *token;
> +int i;
> +
> +if (sscanf(xsval[XS_WATCH_TOKEN], "%li", (unsigned long *)&token) != 1) {
> + xs_set_error(EINVAL);

Please fix indentation here.

With the comments addressed:

Reviewed-by: Wei Liu 

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

Re: [Xen-devel] [PATCH 3/3] python: Add binding for non-blocking xs_check_watch()

[Wei Liu]

On Fri, Sep 15, 2017 at 05:35:36PM +0100, Euan Harris wrote:
> xs_check_watch() checks for watch notifications without blocking.
> Together with the binding for xs_fileno(), this makes it possible
> to write event-driven clients in Python.
> 
> Signed-off-by: Euan Harris 

Reviewed-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v4 03/11] livepatch: Include sizes when an mismatch occurs

[Jan Beulich]

>>> On 21.09.17 at 00:31,  wrote:
> If the .bug.frames.X or .livepatch.funcs sizes are different
> than what the hypervisor expects - we fail the payload. To help
> in diagnosing this include the expected and the payload
> sizes.
> 
> Also make it more natural by having "Multiples" in the warning.
> 
> Also fix one case where we would fail if the size of the .ex_table
> was being zero - but that is OK.
> 
> Signed-off-by: Konrad Rzeszutek Wilk 

Reviewed-by: Jan Beulich 



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

Re: [Xen-devel] [PATCH v4 05/11] alternative/x86/arm32: Align altinstructions (and altinstr_replacement) sections.

[Jan Beulich]

>>> On 21.09.17 at 00:31,  wrote:
> @@ -73,6 +75,7 @@ int apply_alternatives(const struct alt_instr *start, const 
> struct alt_instr *en
>  #include 
>  
>  .macro altinstruction_entry orig_offset alt_offset feature orig_len alt_len
> + .p2align 2
>   .word \orig_offset - .
>   .word \alt_offset - .
>   .hword \feature
> @@ -103,6 +106,7 @@ int apply_alternatives(const struct alt_instr *start, 
> const struct alt_instr *en
>  .macro alternative_if_not cap, enable = 1
>   .if \enable
>   .pushsection .altinstructions, "a"
> + .p2align 2
>   altinstruction_entry 661f, 663f, \cap, 662f-661f, 664f-663f
>   .popsection

Why? altinstruction_entry already does what you want.

x86 parts
Acked-by: Jan Beulich 

Jan


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

Re: [Xen-devel] [PATCH v4 08/11] livepatch/arm/x86: Rename note_depends symbol from test-cases.

[Jan Beulich]

>>> On 21.09.17 at 00:31,  wrote:
> --- a/xen/test/livepatch/Makefile
> +++ b/xen/test/livepatch/Makefile
> @@ -53,6 +53,7 @@ xen_hello_world.o: config.h livepatch_depends.h
>  .PHONY: $(LIVEPATCH)
>  $(LIVEPATCH): xen_hello_world_func.o xen_hello_world.o
>   $(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH) $^
> + $(OBJCOPY) --redefine-sym $(NOTE_SYMBOL)=$@_$(NOTE_SYMBOL) $@

I should have paid attention to this earlier: This model of modifying
in place the target file of a rule doesn't work well when taking into
account someone hitting Ctrl-C in the middle of the build process.
The linker output should go to an intermediate file, and objcopy
should then produce the final one.

Jan


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

Re: [Xen-devel] [PATCH v4 10/11] livepatch/arm[32, 64]: Modify .livepatch.funcs section to be RW when patching

[Jan Beulich]

>>> On 21.09.17 at 00:31,  wrote:
> @@ -43,7 +46,29 @@ int arch_livepatch_quiesce(void)
>  return -ENOMEM;
>  }
>  
> -return 0;
> +if ( nfuncs )
> +{
> +unsigned long va = (unsigned long)func;
> +unsigned int offs = va & (PAGE_SIZE - 1);
> +unsigned int pages = PFN_UP(offs + nfuncs * sizeof(*func));
> +
> +va &= PAGE_MASK;
> +
> +rc = modify_xen_mappings(va, va + (pages * PAGE_SIZE), PTE_NX);
> +if ( rc )
> +{
> +printk(XENLOG_ERR LIVEPATCH "Failed to modify 0x%lx to RW\n", 
> va);

%#lx ?

> +vunmap(vmap_of_xen_text);
> +vmap_of_xen_text = NULL;
> +}
> +else
> +{
> +livepatch_stash.va = va;
> +livepatch_stash.pages = pages;
> +}
> +}

You're effectively doing all this behind the back of vmalloc() / vmap();
I'm not sure this is a good idea, but I'm also not a maintainer of this
code.

Jan


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

[Xen-devel] [linux-linus test] 113650: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-examine  5 xen-install  fail REGR. vs. 113629
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-localmigrate/x10 fail REGR. vs. 
113629

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 113629
 test-amd64-i386-xl-qemut-win7-amd64 18 guest-start/win.repeat fail like 113629
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 113629
 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail  like 113629
 test-amd64-amd64-xl-rtds 10 debian-install   fail  like 113629
 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeatfail  like 113629
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 113629
 test-amd64-amd64-xl-qemut-ws16-amd64 10 windows-installfail never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-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-i386-libvirt-qcow2 12 migrate-support-checkfail  never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   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-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail 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-qemuu-ws16-amd64 13 guest-saverestore   fail never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-installfail 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-qemut-ws16-amd64 13 guest-saverestore   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-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-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-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass

version targeted for testing:
 linuxc52f56a69d104d5294af3d652776d94b1ef6a175
baseline version:
 linux820bf5c419e4b85298e5c3001bd1b5be46d60765

Last test of basis   113629  2017-09-20 12:47:37 Z0 days
Testing same since   113650  2017-09-21 02:05:20 Z0 days1 attempts


People who touched revisions under test:
  Bo Yan 
  Linus Torvalds 
  Steven Rostedt (VMware) 
  Tahsin Erdogan 
  Ziqian SUN (Zamir) 

jobs:
 build-amd64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt  

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

[osstest service owner]

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

Regressions :-(

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

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a

version targeted for testing:
 ovmf 560a435df02b233ea33ae543aeab76b2201de849
baseline version:
 ovmf 947f3737abf65fda63f3ffd97fddfa6986986868

Last test of basis   113647  2017-09-20 22:34:05 Z0 days
Failing since113654  2017-09-21 06:22:39 Z0 days3 attempts
Testing same since   113658  2017-09-21 09:21:11 Z0 days2 attempts


People who touched revisions under test:
  Dandan Bi 
  Hao Wu 
  Jian J Wang 

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



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.


commit 560a435df02b233ea33ae543aeab76b2201de849
Author: Dandan Bi 
Date:   Wed Sep 20 20:19:04 2017 +0800

MdeModulePkg/SetupBrowser: Handle questions with Bit VarStore

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=545

For oneof/numeric/CheckBox(storage can be Bit VarStore)
If the question value can be updated and shown correctly
in UI page, we need do enhancements in following cases:
1. Parse the Ifr data to get the bit VarStore info correctly.
2. Set/get value to/from bit VarStore correctly.

Cc: Eric Dong 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
Reviewed-by: Eric Dong 
Reviewed-by: Liming Gao 

commit 37cd16ac57fcbe5f6ecd15f85ea51621d08cde59
Author: Dandan Bi 
Date:   Wed Sep 20 20:09:04 2017 +0800

MdeModulePkg/HiiDatabase: Handle questions with Bit VarStore

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=545

For oneof/numeric/checkbox, their storage may be bit field.
When generating  string to get default value
for these questions, we need to parse the Ifr data to get
the bit Varstore info,and then generating the correct
 string.

Cc: Eric Dong 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
Reviewed-by: Eric Dong 
Reviewed-by: Liming Gao 

commit 95a71351919aee15874b748f1aae2f8f492d2f76
Author: Dandan Bi 
Date:   Wed Sep 20 19:43:45 2017 +0800

MdeModulePkg/UefiHiiLib: Validate question with bit fields

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=545

In UefiHiiLib, there are codes to validate the current setting of
questions, now update the logic to handle question with bit storage.

Cc: Eric Dong 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
Reviewed-by: Eric Dong 
Reviewed-by: Liming Gao 

commit 01723271a8baef1dc4c92bce0c09d41055cc5eb9
Author: Dandan Bi 
Date:   Wed Sep 20 19:24:18 2017 +0800

MdeModulePkg: Add GUID/flags to implement BitField support

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=545

[Xen-devel] [PATCH v2 01/16] xen/x86: p2m-pod: Clean-up includes

[Julien Grall]

A lot of the headers are not necessary. At the same time, order them in the
alphabetical order.

Signed-off-by: Julien Grall 
Acked-by: Andrew Cooper 

---

Cc: George Dunlap 
Cc: Jan Beulich 
Cc: Andrew Cooper 

Changes in v2:
- Add Andrew's acked-by
---
 xen/arch/x86/mm/p2m-pod.c | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/mm/p2m-pod.c b/xen/arch/x86/mm/p2m-pod.c
index 4085b7f752..fec87e5224 100644
--- a/xen/arch/x86/mm/p2m-pod.c
+++ b/xen/arch/x86/mm/p2m-pod.c
@@ -19,18 +19,13 @@
  * along with this program; If not, see .
  */
 
-#include 
-#include 
 #include 
-#include 
-#include 
+#include 
+#include 
+#include 
 #include 
 #include 
 #include 
-#include  /* ept_p2m_init() */
-#include 
-#include 
-#include 
 
 #include "mm-locks.h"
 
-- 
2.11.0


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

[Xen-devel] [PATCH v2 05/16] xen/x86: p2m-pod: Avoid redundant assignments in p2m_pod_demand_populate

[Julien Grall]

gfn_aligned is assigned 3 times with the exact same formula. All the
variables used are not modified, so consolidate in a single assignment
at the beginning of the function.

Signed-off-by: Julien Grall 
Acked-by: Andrew Cooper 

---

Cc: George Dunlap 
Cc: Jan Beulich 
Cc: Andrew Cooper 

Changes in v2:
- Add Andrew's acked-by
---
 xen/arch/x86/mm/p2m-pod.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/xen/arch/x86/mm/p2m-pod.c b/xen/arch/x86/mm/p2m-pod.c
index f04d6e03e2..bcc87aee03 100644
--- a/xen/arch/x86/mm/p2m-pod.c
+++ b/xen/arch/x86/mm/p2m-pod.c
@@ -1079,7 +1079,7 @@ p2m_pod_demand_populate(struct p2m_domain *p2m, unsigned 
long gfn,
 {
 struct domain *d = p2m->domain;
 struct page_info *p = NULL; /* Compiler warnings */
-unsigned long gfn_aligned;
+unsigned long gfn_aligned = (gfn >> order) << order;
 mfn_t mfn;
 unsigned long i;
 
@@ -1102,7 +1102,6 @@ p2m_pod_demand_populate(struct p2m_domain *p2m, unsigned 
long gfn,
 if ( order == PAGE_ORDER_1G )
 {
 pod_unlock(p2m);
-gfn_aligned = (gfn >> order) << order;
 /*
  * Note that we are supposed to call p2m_set_entry() 512 times to
  * split 1GB into 512 2MB pages here. But We only do once here because
@@ -1147,8 +1146,6 @@ p2m_pod_demand_populate(struct p2m_domain *p2m, unsigned 
long gfn,
 
 BUG_ON((mfn_x(mfn) & ((1UL << order) - 1)) != 0);
 
-gfn_aligned = (gfn >> order) << order;
-
 p2m_set_entry(p2m, gfn_aligned, mfn, order, p2m_ram_rw,
   p2m->default_access);
 
@@ -1200,7 +1197,6 @@ remap_and_retry:
  * NOTE: In a p2m fine-grained lock scenario this might
  * need promoting the gfn lock from gfn->2M superpage.
  */
-gfn_aligned = (gfn >> order) << order;
 for ( i = 0; i < (1UL << order); i++ )
 p2m_set_entry(p2m, gfn_aligned + i, INVALID_MFN, PAGE_ORDER_4K,
   p2m_populate_on_demand, p2m->default_access);
-- 
2.11.0


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

[Xen-devel] [PATCH v2 11/16] xen/x86: p2m-pod: Clean-up p2m_pod_zero_check

[Julien Grall]

Signed-off-by: Julien Grall 
Acked-by: Andrew Cooper 

---

Cc: George Dunlap 
Cc: Jan Beulich 
Cc: Andrew Cooper 

Changes in v2:
- Add Andrew's acked-by
---
 xen/arch/x86/mm/p2m-pod.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/mm/p2m-pod.c b/xen/arch/x86/mm/p2m-pod.c
index 176d06cb42..611a087855 100644
--- a/xen/arch/x86/mm/p2m-pod.c
+++ b/xen/arch/x86/mm/p2m-pod.c
@@ -861,17 +861,19 @@ p2m_pod_zero_check(struct p2m_domain *p2m, unsigned long 
*gfns, int count)
 for ( i = 0; i < count; i++ )
 {
 p2m_access_t a;
+struct page_info *pg;
 
 mfns[i] = p2m->get_entry(p2m, _gfn(gfns[i]), types + i, &a,
  0, NULL, NULL);
+pg = mfn_to_page(mfns[i]);
+
 /*
  * If this is ram, and not a pagetable or from the xen heap, and
  * probably not mapped elsewhere, map it; otherwise, skip.
  */
-if ( p2m_is_ram(types[i])
- && ( (mfn_to_page(mfns[i])->count_info & PGC_allocated) != 0 )
- && ( (mfn_to_page(mfns[i])->count_info & 
(PGC_page_table|PGC_xen_heap)) == 0 )
- && ( (mfn_to_page(mfns[i])->count_info & PGC_count_mask) <= 
max_ref ) )
+if ( p2m_is_ram(types[i]) && (pg->count_info & PGC_allocated) &&
+ ((pg->count_info & (PGC_page_table | PGC_xen_heap)) == 0) &&
+ ((pg->count_info & (PGC_count_mask)) <= max_ref) )
 map[i] = map_domain_page(mfns[i]);
 else
 map[i] = NULL;
-- 
2.11.0


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

[Xen-devel] [PATCH v2 09/16] xen/x86: p2m: Use typesafe GFN in p2m_set_entry

[Julien Grall]

Signed-off-by: Julien Grall 
Acked-by: Andrew Cooper 
Acked-by: Tamas K Lengyel 

---

Cc: George Dunlap 
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Tamas K Lengyel 

Changes in v2:
- Add Andrew & Tamas' acked-by
- Rename the variable gfn_t to gfn_ to avoid shadowing the type
gfn_t
---
 xen/arch/x86/mm/hap/nested_hap.c |   2 +-
 xen/arch/x86/mm/mem_sharing.c|   3 +-
 xen/arch/x86/mm/p2m-pod.c|  36 +++--
 xen/arch/x86/mm/p2m.c| 112 ++-
 xen/include/asm-x86/p2m.h|   2 +-
 5 files changed, 85 insertions(+), 70 deletions(-)

diff --git a/xen/arch/x86/mm/hap/nested_hap.c b/xen/arch/x86/mm/hap/nested_hap.c
index 162afed46b..346fcb53e5 100644
--- a/xen/arch/x86/mm/hap/nested_hap.c
+++ b/xen/arch/x86/mm/hap/nested_hap.c
@@ -121,7 +121,7 @@ nestedhap_fix_p2m(struct vcpu *v, struct p2m_domain *p2m,
 gfn = (L2_gpa >> PAGE_SHIFT) & mask;
 mfn = _mfn((L0_gpa >> PAGE_SHIFT) & mask);
 
-rc = p2m_set_entry(p2m, gfn, mfn, page_order, p2mt, p2ma);
+rc = p2m_set_entry(p2m, _gfn(gfn), mfn, page_order, p2mt, p2ma);
 }
 
 p2m_unlock(p2m);
diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index 62a3899089..b856028c02 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -1052,7 +1052,8 @@ int mem_sharing_add_to_physmap(struct domain *sd, 
unsigned long sgfn, shr_handle
 goto err_unlock;
 }
 
-ret = p2m_set_entry(p2m, cgfn, smfn, PAGE_ORDER_4K, p2m_ram_shared, a);
+ret = p2m_set_entry(p2m, _gfn(cgfn), smfn, PAGE_ORDER_4K,
+p2m_ram_shared, a);
 
 /* Tempted to turn this into an assert */
 if ( ret )
diff --git a/xen/arch/x86/mm/p2m-pod.c b/xen/arch/x86/mm/p2m-pod.c
index c8c8cff014..b8a51cf12a 100644
--- a/xen/arch/x86/mm/p2m-pod.c
+++ b/xen/arch/x86/mm/p2m-pod.c
@@ -565,7 +565,7 @@ p2m_pod_decrease_reservation(struct domain *d, gfn_t gfn, 
unsigned int order)
  * All PoD: Mark the whole region invalid and tell caller
  * we're done.
  */
-p2m_set_entry(p2m, gfn_x(gfn), INVALID_MFN, order, p2m_invalid,
+p2m_set_entry(p2m, gfn, INVALID_MFN, order, p2m_invalid,
   p2m->default_access);
 p2m->pod.entry_count -= 1UL << order;
 BUG_ON(p2m->pod.entry_count < 0);
@@ -609,7 +609,7 @@ p2m_pod_decrease_reservation(struct domain *d, gfn_t gfn, 
unsigned int order)
 n = 1UL << cur_order;
 if ( t == p2m_populate_on_demand )
 {
-p2m_set_entry(p2m, gfn_x(gfn) + i, INVALID_MFN, cur_order,
+p2m_set_entry(p2m, gfn_add(gfn, i), INVALID_MFN, cur_order,
   p2m_invalid, p2m->default_access);
 p2m->pod.entry_count -= n;
 BUG_ON(p2m->pod.entry_count < 0);
@@ -631,7 +631,7 @@ p2m_pod_decrease_reservation(struct domain *d, gfn_t gfn, 
unsigned int order)
 
 page = mfn_to_page(mfn);
 
-p2m_set_entry(p2m, gfn_x(gfn) + i, INVALID_MFN, cur_order,
+p2m_set_entry(p2m, gfn_add(gfn, i), INVALID_MFN, cur_order,
   p2m_invalid, p2m->default_access);
 p2m_tlb_flush_sync(p2m);
 for ( j = 0; j < n; ++j )
@@ -680,9 +680,10 @@ void p2m_pod_dump_data(struct domain *d)
  * in the p2m.
  */
 static int
-p2m_pod_zero_check_superpage(struct p2m_domain *p2m, unsigned long gfn)
+p2m_pod_zero_check_superpage(struct p2m_domain *p2m, unsigned long gfn_l)
 {
 mfn_t mfn, mfn0 = INVALID_MFN;
+gfn_t gfn = _gfn(gfn_l);
 p2m_type_t type, type0 = 0;
 unsigned long * map = NULL;
 int ret=0, reset = 0;
@@ -693,7 +694,7 @@ p2m_pod_zero_check_superpage(struct p2m_domain *p2m, 
unsigned long gfn)
 
 ASSERT(pod_locked_by_me(p2m));
 
-if ( !superpage_aligned(gfn) )
+if ( !superpage_aligned(gfn_l) )
 goto out;
 
 /* Allow an extra refcount for one shadow pt mapping in shadowed domains */
@@ -717,7 +718,7 @@ p2m_pod_zero_check_superpage(struct p2m_domain *p2m, 
unsigned long gfn)
 unsigned long k;
 const struct page_info *page;
 
-mfn = p2m->get_entry(p2m, _gfn(gfn +  i), &type, &a, 0,
+mfn = p2m->get_entry(p2m, gfn_add(gfn, i), &type, &a, 0,
  &cur_order, NULL);
 
 /*
@@ -815,7 +816,7 @@ p2m_pod_zero_check_superpage(struct p2m_domain *p2m, 
unsigned long gfn)
 int d:16,order:16;
 } t;
 
-t.gfn = gfn;
+t.gfn = gfn_l;
 t.mfn = mfn_x(mfn);
 t.d = d->domain_id;
 t.order = 9;
@@ -898,7 +899,7 @@ p2m_pod_zero_check(struct p2m_domain *p2m, unsigned long 
*gfns, int count)
 }
 
 /* Try to remove the page, restoring old mapping if it fails. */
-p2m_set_entry(p2m, gfns[i], INVALID_MFN, PAGE_ORDER_4K,
+p2m_set_entry(p2m, _gfn(gfns[i]), INVALID_MFN, PAGE_ORDER_4K,
   p2m_populate_on_demand, p2m-

[Xen-devel] [PATCH v2 15/16] xen/x86: p2m-pod: Rework prototype of p2m_pod_demand_populate

[Julien Grall]

- Switch the return type to bool
- Remove the parameter p2m_query_t q as it is not used

Signed-off-by: Julien Grall 
Acked-by: Andrew Cooper 

---

Cc: George Dunlap 
Cc: Jan Beulich 
Cc: Andrew Cooper 

Changes in v2:
- Add Andrew's acked-by
---
 xen/arch/x86/mm/p2m-ept.c |  4 ++--
 xen/arch/x86/mm/p2m-pod.c | 15 +++
 xen/arch/x86/mm/p2m-pt.c  |  6 +++---
 xen/include/asm-x86/p2m.h |  6 ++
 4 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
index bc25582c5a..054827aa88 100644
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -965,7 +965,7 @@ static mfn_t ept_get_entry(struct p2m_domain *p2m,
 index = gfn_remainder >> ( i * EPT_TABLE_ORDER);
 ept_entry = table + index;
 
-if ( !p2m_pod_demand_populate(p2m, gfn_, i * EPT_TABLE_ORDER, q) )
+if ( p2m_pod_demand_populate(p2m, gfn_, i * EPT_TABLE_ORDER) )
 goto retry;
 else
 goto out;
@@ -987,7 +987,7 @@ static mfn_t ept_get_entry(struct p2m_domain *p2m,
 
 ASSERT(i == 0);
 
-if ( p2m_pod_demand_populate(p2m, gfn_, PAGE_ORDER_4K, q) )
+if ( !p2m_pod_demand_populate(p2m, gfn_, PAGE_ORDER_4K) )
 goto out;
 }
 
diff --git a/xen/arch/x86/mm/p2m-pod.c b/xen/arch/x86/mm/p2m-pod.c
index 311762b1ce..89b7dc949d 100644
--- a/xen/arch/x86/mm/p2m-pod.c
+++ b/xen/arch/x86/mm/p2m-pod.c
@@ -1075,10 +1075,9 @@ static void pod_eager_record(struct p2m_domain *p2m, 
gfn_t gfn,
 mrp->idx %= ARRAY_SIZE(mrp->list);
 }
 
-int
+bool
 p2m_pod_demand_populate(struct p2m_domain *p2m, gfn_t gfn,
-unsigned int order,
-p2m_query_t q)
+unsigned int order)
 {
 struct domain *d = p2m->domain;
 struct page_info *p = NULL; /* Compiler warnings */
@@ -1116,7 +1115,7 @@ p2m_pod_demand_populate(struct p2m_domain *p2m, gfn_t gfn,
  */
 p2m_set_entry(p2m, gfn_aligned, INVALID_MFN, PAGE_ORDER_2M,
   p2m_populate_on_demand, p2m->default_access);
-return 0;
+return true;
 }
 
 /* Only reclaim if we're in actual need of more cache. */
@@ -1178,7 +1177,7 @@ p2m_pod_demand_populate(struct p2m_domain *p2m, gfn_t gfn,
 }
 
 pod_unlock(p2m);
-return 0;
+return true;
 out_of_memory:
 pod_unlock(p2m);
 
@@ -1186,10 +1185,10 @@ out_of_memory:
__func__, d->domain_id, d->tot_pages, p2m->pod.entry_count,
current->domain->domain_id);
 domain_crash(d);
-return -1;
+return false;
 out_fail:
 pod_unlock(p2m);
-return -1;
+return false;
 remap_and_retry:
 BUG_ON(order != PAGE_ORDER_2M);
 pod_unlock(p2m);
@@ -1215,7 +1214,7 @@ remap_and_retry:
 __trace_var(TRC_MEM_POD_SUPERPAGE_SPLINTER, 0, sizeof(t), &t);
 }
 
-return 0;
+return true;
 }
 
 
diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c
index a639a00e9c..50637083f4 100644
--- a/xen/arch/x86/mm/p2m-pt.c
+++ b/xen/arch/x86/mm/p2m-pt.c
@@ -802,7 +802,7 @@ pod_retry_l3:
 {
 if ( q & P2M_ALLOC )
 {
-if ( !p2m_pod_demand_populate(p2m, gfn_, PAGE_ORDER_1G, q) 
)
+if ( p2m_pod_demand_populate(p2m, gfn_, PAGE_ORDER_1G) )
 goto pod_retry_l3;
 gdprintk(XENLOG_ERR, "%s: Allocate 1GB failed!\n", 
__func__);
 }
@@ -844,7 +844,7 @@ pod_retry_l2:
 if ( p2m_flags_to_type(flags) == p2m_populate_on_demand )
 {
 if ( q & P2M_ALLOC ) {
-if ( !p2m_pod_demand_populate(p2m, gfn_, PAGE_ORDER_2M, q) )
+if ( p2m_pod_demand_populate(p2m, gfn_, PAGE_ORDER_2M) )
 goto pod_retry_l2;
 } else
 *t = p2m_populate_on_demand;
@@ -883,7 +883,7 @@ pod_retry_l1:
 if ( l1t == p2m_populate_on_demand )
 {
 if ( q & P2M_ALLOC ) {
-if ( !p2m_pod_demand_populate(p2m, gfn_, PAGE_ORDER_4K, q) )
+if ( p2m_pod_demand_populate(p2m, gfn_, PAGE_ORDER_4K) )
 goto pod_retry_l1;
 } else
 *t = p2m_populate_on_demand;
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index e8a9dca480..70f00c332f 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -719,10 +719,8 @@ extern void audit_p2m(struct domain *d,
 #endif
 
 /* Called by p2m code when demand-populating a PoD page */
-int
-p2m_pod_demand_populate(struct p2m_domain *p2m, gfn_t gfn,
-unsigned int order,
-p2m_query_t q);
+bool
+p2m_pod_demand_populate(struct p2m_domain *p2m, gfn_t gfn, unsigned int order);
 
 /*
  * Functions specific to the p2m-pt implementation
-- 
2.11.0


___
Xe

[Xen-devel] [PATCH v2 07/16] xen/x86: p2m-pod: Use typesafe gfn in p2m_pod_decrease_reservation

[Julien Grall]

Signed-off-by: Julien Grall 
Acked-by: Andrew Cooper 

---

Cc: George Dunlap 
Cc: Jan Beulich 
Cc: Andrew Cooper 

Changes in v2:
- Add Andrew's acked-by
---
 xen/arch/arm/p2m.c   |  3 +--
 xen/arch/x86/mm/p2m-pod.c| 20 +---
 xen/common/memory.c  |  3 ++-
 xen/include/asm-arm/p2m.h| 13 -
 xen/include/asm-x86/p2m.h|  7 ---
 xen/include/xen/p2m-common.h | 13 +
 6 files changed, 25 insertions(+), 34 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 192a1c329d..0410b1e86b 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -393,8 +393,7 @@ int guest_physmap_mark_populate_on_demand(struct domain *d,
 return -ENOSYS;
 }
 
-int p2m_pod_decrease_reservation(struct domain *d,
- xen_pfn_t gpfn,
+int p2m_pod_decrease_reservation(struct domain *d, gfn_t gfn,
  unsigned int order)
 {
 return -ENOSYS;
diff --git a/xen/arch/x86/mm/p2m-pod.c b/xen/arch/x86/mm/p2m-pod.c
index 34f5239b6d..eb74e5c01f 100644
--- a/xen/arch/x86/mm/p2m-pod.c
+++ b/xen/arch/x86/mm/p2m-pod.c
@@ -511,9 +511,7 @@ p2m_pod_zero_check_superpage(struct p2m_domain *p2m, 
unsigned long gfn);
  * allow decrease_reservation() to handle everything else.
  */
 int
-p2m_pod_decrease_reservation(struct domain *d,
- xen_pfn_t gpfn,
- unsigned int order)
+p2m_pod_decrease_reservation(struct domain *d, gfn_t gfn, unsigned int order)
 {
 int ret = 0;
 unsigned long i, n;
@@ -521,7 +519,7 @@ p2m_pod_decrease_reservation(struct domain *d,
 bool_t steal_for_cache;
 long pod, nonpod, ram;
 
-gfn_lock(p2m, gpfn, order);
+gfn_lock(p2m, gfn, order);
 pod_lock(p2m);
 
 /*
@@ -545,7 +543,7 @@ p2m_pod_decrease_reservation(struct domain *d,
 p2m_type_t t;
 unsigned int cur_order;
 
-p2m->get_entry(p2m, gpfn + i, &t, &a, 0, &cur_order, NULL);
+p2m->get_entry(p2m, gfn_x(gfn) + i, &t, &a, 0, &cur_order, NULL);
 n = 1UL << min(order, cur_order);
 if ( t == p2m_populate_on_demand )
 pod += n;
@@ -567,7 +565,7 @@ p2m_pod_decrease_reservation(struct domain *d,
  * All PoD: Mark the whole region invalid and tell caller
  * we're done.
  */
-p2m_set_entry(p2m, gpfn, INVALID_MFN, order, p2m_invalid,
+p2m_set_entry(p2m, gfn_x(gfn), INVALID_MFN, order, p2m_invalid,
   p2m->default_access);
 p2m->pod.entry_count -= 1UL << order;
 BUG_ON(p2m->pod.entry_count < 0);
@@ -584,7 +582,7 @@ p2m_pod_decrease_reservation(struct domain *d,
  * - not all of the pages were RAM (now knowing order < SUPERPAGE_ORDER)
  */
 if ( steal_for_cache && order < SUPERPAGE_ORDER && ram == (1UL << order) &&
- p2m_pod_zero_check_superpage(p2m, gpfn & ~(SUPERPAGE_PAGES - 1)) )
+ p2m_pod_zero_check_superpage(p2m, gfn_x(gfn) & ~(SUPERPAGE_PAGES - 
1)) )
 {
 pod = 1UL << order;
 ram = nonpod = 0;
@@ -605,13 +603,13 @@ p2m_pod_decrease_reservation(struct domain *d,
 p2m_access_t a;
 unsigned int cur_order;
 
-mfn = p2m->get_entry(p2m, gpfn + i, &t, &a, 0, &cur_order, NULL);
+mfn = p2m->get_entry(p2m, gfn_x(gfn) + i, &t, &a, 0, &cur_order, NULL);
 if ( order < cur_order )
 cur_order = order;
 n = 1UL << cur_order;
 if ( t == p2m_populate_on_demand )
 {
-p2m_set_entry(p2m, gpfn + i, INVALID_MFN, cur_order,
+p2m_set_entry(p2m, gfn_x(gfn) + i, INVALID_MFN, cur_order,
   p2m_invalid, p2m->default_access);
 p2m->pod.entry_count -= n;
 BUG_ON(p2m->pod.entry_count < 0);
@@ -633,7 +631,7 @@ p2m_pod_decrease_reservation(struct domain *d,
 
 page = mfn_to_page(mfn);
 
-p2m_set_entry(p2m, gpfn + i, INVALID_MFN, cur_order,
+p2m_set_entry(p2m, gfn_x(gfn) + i, INVALID_MFN, cur_order,
   p2m_invalid, p2m->default_access);
 p2m_tlb_flush_sync(p2m);
 for ( j = 0; j < n; ++j )
@@ -663,7 +661,7 @@ out_entry_check:
 
 out_unlock:
 pod_unlock(p2m);
-gfn_unlock(p2m, gpfn, order);
+gfn_unlock(p2m, gfn, order);
 return ret;
 }
 
diff --git a/xen/common/memory.c b/xen/common/memory.c
index a2abf554e3..ad987e0f29 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -417,7 +417,8 @@ static void decrease_reservation(struct memop_args *a)
 
 /* See if populate-on-demand wants to handle this */
 if ( is_hvm_domain(a->domain)
- && p2m_pod_decrease_reservation(a->domain, gmfn, a->extent_order) 
)
+ && p2m_pod_decrease_reservation(a->domain, _gfn(gmfn),
+ a->extent_order) )
 continue;
 
 for ( j = 0; j < (1 << a->extent_order); j++ )
diff --git a/

[Xen-devel] [PATCH v2 14/16] xen/x86: p2m-pod: Use typesafe gfn for the fields reclaim_single and max_guest

[Julien Grall]

Signed-off-by: Julien Grall 
Acked-by: Andrew Cooper 

---

Cc: George Dunlap 
Cc: Jan Beulich 
Cc: Andrew Cooper 

Changes in v2:
- Add Andrew's acked-by
---
 xen/arch/x86/mm/p2m-pod.c | 11 +--
 xen/include/asm-x86/p2m.h |  4 ++--
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/mm/p2m-pod.c b/xen/arch/x86/mm/p2m-pod.c
index 5c79444d7b..311762b1ce 100644
--- a/xen/arch/x86/mm/p2m-pod.c
+++ b/xen/arch/x86/mm/p2m-pod.c
@@ -977,10 +977,10 @@ p2m_pod_emergency_sweep(struct p2m_domain *p2m)
 p2m_type_t t;
 
 
-if ( p2m->pod.reclaim_single == 0 )
+if ( gfn_eq(p2m->pod.reclaim_single, _gfn(0)) )
 p2m->pod.reclaim_single = p2m->pod.max_guest;
 
-start = p2m->pod.reclaim_single;
+start = gfn_x(p2m->pod.reclaim_single);
 limit = (start > POD_SWEEP_LIMIT) ? (start - POD_SWEEP_LIMIT) : 0;
 
 /* FIXME: Figure out how to avoid superpages */
@@ -990,7 +990,7 @@ p2m_pod_emergency_sweep(struct p2m_domain *p2m)
  * careful about spinlock recursion limits and POD_SWEEP_STRIDE.
  */
 p2m_lock(p2m);
-for ( i = p2m->pod.reclaim_single; i > 0 ; i-- )
+for ( i = gfn_x(p2m->pod.reclaim_single); i > 0 ; i-- )
 {
 p2m_access_t a;
 (void)p2m->get_entry(p2m, _gfn(i), &t, &a, 0, NULL, NULL);
@@ -1020,7 +1020,7 @@ p2m_pod_emergency_sweep(struct p2m_domain *p2m)
 p2m_pod_zero_check(p2m, gfns, j);
 
 p2m_unlock(p2m);
-p2m->pod.reclaim_single = i ? i - 1 : i;
+p2m->pod.reclaim_single = _gfn(i ? i - 1 : i);
 
 }
 
@@ -1135,8 +1135,7 @@ p2m_pod_demand_populate(struct p2m_domain *p2m, gfn_t gfn,
 goto out_of_memory;
 
 /* Keep track of the highest gfn demand-populated by a guest fault */
-if ( gfn_x(gfn) > p2m->pod.max_guest )
-p2m->pod.max_guest = gfn_x(gfn);
+p2m->pod.max_guest = gfn_max(gfn, p2m->pod.max_guest);
 
 /*
  * Get a page f/ the cache.  A NULL return value indicates that the
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index 1ae9216404..e8a9dca480 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -316,8 +316,8 @@ struct p2m_domain {
  single;   /* Non-super lists   */
 long count,/* # of pages in cache lists */
  entry_count;  /* # of pages in p2m marked pod  */
-unsigned longreclaim_single; /* Last gpfn of a scan */
-unsigned longmax_guest;/* gpfn of max guest demand-populate */
+gfn_treclaim_single; /* Last gfn of a scan */
+gfn_tmax_guest;/* gfn of max guest demand-populate */
 
 /*
  * Tracking of the most recently populated PoD pages, for eager
-- 
2.11.0


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

[Xen-devel] [PATCH v2 08/16] xen/x86: p2m: Use typesafe gfn for the P2M callbacks get_entry and set_entry

[Julien Grall]

Signed-off-by: Julien Grall 
Acked-by: Andrew Cooper 
Reviewed-by: Kevin Tian 

---

Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Razvan Cojocaru 
Cc: Tamas K Lengyel 
Cc: George Dunlap 
Cc: Jun Nakajima 
Cc: Kevin Tian 

Changes in v2:
- Add Andre's acked
- Add Kevin's reviewed (EPT part)
---
 xen/arch/x86/hvm/hvm.c|  2 +-
 xen/arch/x86/mm/mem_access.c  | 19 +--
 xen/arch/x86/mm/mem_sharing.c |  4 +--
 xen/arch/x86/mm/p2m-ept.c |  6 ++--
 xen/arch/x86/mm/p2m-pod.c | 15 +
 xen/arch/x86/mm/p2m-pt.c  |  6 ++--
 xen/arch/x86/mm/p2m.c | 77 +++
 xen/include/asm-x86/p2m.h |  4 +--
 8 files changed, 73 insertions(+), 60 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 93394c1fb6..2a32102a41 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1787,7 +1787,7 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned long 
gla,
 {
 bool_t sve;
 
-p2m->get_entry(p2m, gfn, &p2mt, &p2ma, 0, NULL, &sve);
+p2m->get_entry(p2m, _gfn(gfn), &p2mt, &p2ma, 0, NULL, &sve);
 
 if ( !sve && altp2m_vcpu_emulate_ve(curr) )
 {
diff --git a/xen/arch/x86/mm/mem_access.c b/xen/arch/x86/mm/mem_access.c
index 9211fc0abe..948e377e69 100644
--- a/xen/arch/x86/mm/mem_access.c
+++ b/xen/arch/x86/mm/mem_access.c
@@ -66,7 +66,7 @@ static int _p2m_get_mem_access(struct p2m_domain *p2m, gfn_t 
gfn,
 }
 
 gfn_lock(p2m, gfn, 0);
-mfn = p2m->get_entry(p2m, gfn_x(gfn), &t, &a, 0, NULL, NULL);
+mfn = p2m->get_entry(p2m, gfn, &t, &a, 0, NULL, NULL);
 gfn_unlock(p2m, gfn, 0);
 
 if ( mfn_eq(mfn, INVALID_MFN) )
@@ -142,7 +142,7 @@ bool p2m_mem_access_check(paddr_t gpa, unsigned long gla,
   vm_event_request_t **req_ptr)
 {
 struct vcpu *v = current;
-unsigned long gfn = gpa >> PAGE_SHIFT;
+gfn_t gfn = gaddr_to_gfn(gpa);
 struct domain *d = v->domain;
 struct p2m_domain *p2m = NULL;
 mfn_t mfn;
@@ -215,7 +215,7 @@ bool p2m_mem_access_check(paddr_t gpa, unsigned long gla,
 *req_ptr = req;
 
 req->reason = VM_EVENT_REASON_MEM_ACCESS;
-req->u.mem_access.gfn = gfn;
+req->u.mem_access.gfn = gfn_x(gfn);
 req->u.mem_access.offset = gpa & ((1 << PAGE_SHIFT) - 1);
 if ( npfec.gla_valid )
 {
@@ -247,7 +247,7 @@ int p2m_set_altp2m_mem_access(struct domain *d, struct 
p2m_domain *hp2m,
 unsigned long gfn_l = gfn_x(gfn);
 int rc;
 
-mfn = ap2m->get_entry(ap2m, gfn_l, &t, &old_a, 0, NULL, NULL);
+mfn = ap2m->get_entry(ap2m, gfn, &t, &old_a, 0, NULL, NULL);
 
 /* Check host p2m if no valid entry in alternate */
 if ( !mfn_valid(mfn) )
@@ -264,16 +264,16 @@ int p2m_set_altp2m_mem_access(struct domain *d, struct 
p2m_domain *hp2m,
 if ( page_order != PAGE_ORDER_4K )
 {
 unsigned long mask = ~((1UL << page_order) - 1);
-unsigned long gfn2_l = gfn_l & mask;
+gfn_t gfn2 = _gfn(gfn_l & mask);
 mfn_t mfn2 = _mfn(mfn_x(mfn) & mask);
 
-rc = ap2m->set_entry(ap2m, gfn2_l, mfn2, page_order, t, old_a, 1);
+rc = ap2m->set_entry(ap2m, gfn2, mfn2, page_order, t, old_a, 1);
 if ( rc )
 return rc;
 }
 }
 
-return ap2m->set_entry(ap2m, gfn_l, mfn, PAGE_ORDER_4K, t, a,
+return ap2m->set_entry(ap2m, gfn, mfn, PAGE_ORDER_4K, t, a,
  (current->domain != d));
 }
 
@@ -295,10 +295,9 @@ static int set_mem_access(struct domain *d, struct 
p2m_domain *p2m,
 mfn_t mfn;
 p2m_access_t _a;
 p2m_type_t t;
-unsigned long gfn_l = gfn_x(gfn);
 
-mfn = p2m->get_entry(p2m, gfn_l, &t, &_a, 0, NULL, NULL);
-rc = p2m->set_entry(p2m, gfn_l, mfn, PAGE_ORDER_4K, t, a, -1);
+mfn = p2m->get_entry(p2m, gfn, &t, &_a, 0, NULL, NULL);
+rc = p2m->set_entry(p2m, gfn, mfn, PAGE_ORDER_4K, t, a, -1);
 }
 
 return rc;
diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index 3ab119cef2..62a3899089 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -1234,7 +1234,7 @@ int relinquish_shared_pages(struct domain *d)
 
 if ( atomic_read(&d->shr_pages) == 0 )
 break;
-mfn = p2m->get_entry(p2m, gfn, &t, &a, 0, NULL, NULL);
+mfn = p2m->get_entry(p2m, _gfn(gfn), &t, &a, 0, NULL, NULL);
 if ( mfn_valid(mfn) && (t == p2m_ram_shared) )
 {
 /* Does not fail with ENOMEM given the DESTROY flag */
@@ -1243,7 +1243,7 @@ int relinquish_shared_pages(struct domain *d)
 /* Clear out the p2m entry so no one else may try to
  * unshare.  Must succeed: we just read the old entry and
  * we hold the p2m lock. */
-set_rc = p2m->set_entry(p2m, gfn, _mfn(0), PAGE_ORDER_4K,
+  

[Xen-devel] [PATCH v2 02/16] xen/x86: p2m-pod: Remove trailing whitespaces

[Julien Grall]

Signed-off-by: Julien Grall 
Acked-by: Andrew Cooper 

---

Cc: George Dunlap 
Cc: Jan Beulich 
Cc: Andrew Cooper 

Changes in v2:
- Add Andrew's acked-by
---
 xen/arch/x86/mm/p2m-pod.c | 46 +++---
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/xen/arch/x86/mm/p2m-pod.c b/xen/arch/x86/mm/p2m-pod.c
index fec87e5224..1f07441259 100644
--- a/xen/arch/x86/mm/p2m-pod.c
+++ b/xen/arch/x86/mm/p2m-pod.c
@@ -1,7 +1,7 @@
 /**
  * arch/x86/mm/p2m-pod.c
  *
- * Populate-on-demand p2m entries. 
+ * Populate-on-demand p2m entries.
  *
  * Copyright (c) 2009-2011 Citrix Systems, Inc.
  *
@@ -76,7 +76,7 @@ p2m_pod_cache_add(struct p2m_domain *p2m,
__func__, mfn_x(mfn), order, ((1UL << order) - 1));
 return -1;
 }
-
+
 for(i=0; i < 1 << order ; i++) {
 struct domain * od;
 
@@ -223,8 +223,8 @@ p2m_pod_set_cache_target(struct p2m_domain *p2m, unsigned 
long pod_target, int p
 /* If we can't allocate a superpage, try singleton pages */
 order = PAGE_ORDER_4K;
 goto retry;
-}   
-
+}
+
 printk("%s: Unable to allocate page for PoD cache (target=%lu 
cache=%ld)\n",
__func__, pod_target, p2m->pod.count);
 ret = -ENOMEM;
@@ -272,7 +272,7 @@ p2m_pod_set_cache_target(struct p2m_domain *p2m, unsigned 
long pod_target, int p
 
 if ( test_and_clear_bit(_PGT_pinned, &(page+i)->u.inuse.type_info) 
)
 put_page_and_type(page+i);
-
+
 if ( test_and_clear_bit(_PGC_allocated, &(page+i)->count_info) )
 put_page(page+i);
 
@@ -296,7 +296,7 @@ out:
  * definitions:
  * + M: static_max
  * + B: number of pages the balloon driver has ballooned down to.
- * + P: Number of populated pages. 
+ * + P: Number of populated pages.
  * + T: Old target
  * + T': New target
  *
@@ -311,10 +311,10 @@ out:
  *   the remainder of the ram to the guest OS.
  *  T default_access);
-
+
 out:
 gfn_unlock(p2m, gfn, SUPERPAGE_ORDER);
 return ret;
@@ -836,8 +836,8 @@ p2m_pod_zero_check(struct p2m_domain *p2m, unsigned long 
*gfns, int count)
 /* If this is ram, and not a pagetable or from the xen heap, and 
probably not mapped
elsewhere, map it; otherwise, skip. */
 if ( p2m_is_ram(types[i])
- && ( (mfn_to_page(mfns[i])->count_info & PGC_allocated) != 0 ) 
- && ( (mfn_to_page(mfns[i])->count_info & 
(PGC_page_table|PGC_xen_heap)) == 0 ) 
+ && ( (mfn_to_page(mfns[i])->count_info & PGC_allocated) != 0 )
+ && ( (mfn_to_page(mfns[i])->count_info & 
(PGC_page_table|PGC_xen_heap)) == 0 )
  && ( (mfn_to_page(mfns[i])->count_info & PGC_count_mask) <= 
max_ref ) )
 map[i] = map_domain_page(mfns[i]);
 else
@@ -915,7 +915,7 @@ p2m_pod_zero_check(struct p2m_domain *p2m, unsigned long 
*gfns, int count)
 t.mfn = mfn_x(mfns[i]);
 t.d = d->domain_id;
 t.order = 0;
-
+
 __trace_var(TRC_MEM_POD_ZERO_RECLAIM, 0, sizeof(t), &t);
 }
 
@@ -924,7 +924,7 @@ p2m_pod_zero_check(struct p2m_domain *p2m, unsigned long 
*gfns, int count)
 p2m->pod.entry_count++;
 }
 }
-
+
 }
 
 #define POD_SWEEP_LIMIT 1024
@@ -1046,12 +1046,12 @@ p2m_pod_demand_populate(struct p2m_domain *p2m, 
unsigned long gfn,
 pod_lock(p2m);
 
 /* This check is done with the pod lock held.  This will make sure that
- * even if d->is_dying changes under our feet, p2m_pod_empty_cache() 
+ * even if d->is_dying changes under our feet, p2m_pod_empty_cache()
  * won't start until we're done. */
 if ( unlikely(d->is_dying) )
 goto out_fail;
 
-
+
 /* Because PoD does not have cache list for 1GB pages, it has to remap
  * 1GB region to 2MB chunks for a retry. */
 if ( order == PAGE_ORDER_1G )
@@ -1107,7 +1107,7 @@ p2m_pod_demand_populate(struct p2m_domain *p2m, unsigned 
long gfn,
 set_gpfn_from_mfn(mfn_x(mfn) + i, gfn_aligned + i);
 paging_mark_dirty(d, mfn_add(mfn, i));
 }
-
+
 p2m->pod.entry_count -= (1 << order);
 BUG_ON(p2m->pod.entry_count < 0);
 
@@ -1124,7 +1124,7 @@ p2m_pod_demand_populate(struct p2m_domain *p2m, unsigned 
long gfn,
 t.mfn = mfn_x(mfn);
 t.d = d->domain_id;
 t.order = order;
-
+
 __trace_var(TRC_MEM_POD_POPULATE, 0, sizeof(t), &t);
 }
 
@@ -1161,7 +1161,7 @@ remap_and_retry:
 
 t.gfn = gfn;
 t.d = d->domain_id;
-
+
 __trace_var(TRC_MEM_POD_SUPERPAGE_SPLINTER, 0, sizeof(t), &t);
 }
 
-- 
2.11.0


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

[Xen-devel] [PATCH v2 06/16] xen/x86: p2m-pod: Clean-up use of typesafe MFN

[Julien Grall]

Some unboxing/boxing can be avoided by using mfn_add(...) instead.

Signed-off-by: Julien Grall 
Acked-by: Andrew Cooper 

---

Cc: George Dunlap 
Cc: Jan Beulich 
Cc: Andrew Cooper 

Changes in v2:
- Add Andrew's acked-by
---
 xen/arch/x86/mm/p2m-pod.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/mm/p2m-pod.c b/xen/arch/x86/mm/p2m-pod.c
index bcc87aee03..34f5239b6d 100644
--- a/xen/arch/x86/mm/p2m-pod.c
+++ b/xen/arch/x86/mm/p2m-pod.c
@@ -101,7 +101,7 @@ p2m_pod_cache_add(struct p2m_domain *p2m,
  * promise to provide zero pages. So we scrub pages before using.
  */
 for ( i = 0; i < (1UL << order); i++ )
-clear_domain_page(_mfn(mfn_x(page_to_mfn(page)) + i));
+clear_domain_page(mfn_add(page_to_mfn(page), i));
 
 /* First, take all pages off the domain list */
 lock_page_alloc(p2m);
@@ -743,7 +743,7 @@ p2m_pod_zero_check_superpage(struct p2m_domain *p2m, 
unsigned long gfn)
 mfn0 = mfn;
 type0 = type;
 }
-else if ( type != type0 || mfn_x(mfn) != (mfn_x(mfn0) + i) )
+else if ( type != type0 || !mfn_eq(mfn, mfn_add(mfn0, i)) )
 goto out;
 
 n = 1UL << min(cur_order, SUPERPAGE_ORDER + 0U);
@@ -758,7 +758,7 @@ p2m_pod_zero_check_superpage(struct p2m_domain *p2m, 
unsigned long gfn)
 for ( i = 0; i < SUPERPAGE_PAGES; i++ )
 {
 /* Quick zero-check */
-map = map_domain_page(_mfn(mfn_x(mfn0) + i));
+map = map_domain_page(mfn_add(mfn0, i));
 
 for ( j = 0; j < 16; j++ )
 if ( *(map + j) != 0 )
@@ -783,7 +783,7 @@ p2m_pod_zero_check_superpage(struct p2m_domain *p2m, 
unsigned long gfn)
  */
 for ( i = 0; i < SUPERPAGE_PAGES; i++ )
 {
-mfn = _mfn(mfn_x(mfn0) + i);
+mfn = mfn_add(mfn0, i);
 if ( (mfn_to_page(mfn)->count_info & PGC_count_mask) > 1 )
 {
 reset = 1;
@@ -794,7 +794,7 @@ p2m_pod_zero_check_superpage(struct p2m_domain *p2m, 
unsigned long gfn)
 /* Finally, do a full zero-check */
 for ( i = 0; i < SUPERPAGE_PAGES; i++ )
 {
-map = map_domain_page(_mfn(mfn_x(mfn0) + i));
+map = map_domain_page(mfn_add(mfn0, i));
 
 for ( j = 0; j < (PAGE_SIZE / sizeof(*map)); j++ )
 if ( *(map+j) != 0 )
-- 
2.11.0


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

[Xen-devel] [PATCH v2 13/16] xen/x86: p2m-pod: Use typesafe gfn in p2m_pod_demand_populate

[Julien Grall]

Signed-off-by: Julien Grall 
Acked-by: Andrew Cooper 

---

Cc: George Dunlap 
Cc: Jan Beulich 
Cc: Andrew Cooper 

Changes in v2:
- Add Andrew's acked-by
- Variable gfn_t was renamed to gfn_ in a previous patch. So use
the new name
---
 xen/arch/x86/mm/p2m-ept.c |  5 ++---
 xen/arch/x86/mm/p2m-pod.c | 12 ++--
 xen/arch/x86/mm/p2m-pt.c  |  6 +++---
 xen/include/asm-x86/p2m.h |  2 +-
 4 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
index f14d1686b7..bc25582c5a 100644
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -965,7 +965,7 @@ static mfn_t ept_get_entry(struct p2m_domain *p2m,
 index = gfn_remainder >> ( i * EPT_TABLE_ORDER);
 ept_entry = table + index;
 
-if ( !p2m_pod_demand_populate(p2m, gfn, i * EPT_TABLE_ORDER, q) )
+if ( !p2m_pod_demand_populate(p2m, gfn_, i * EPT_TABLE_ORDER, q) )
 goto retry;
 else
 goto out;
@@ -987,8 +987,7 @@ static mfn_t ept_get_entry(struct p2m_domain *p2m,
 
 ASSERT(i == 0);
 
-if ( p2m_pod_demand_populate(p2m, gfn, 
-PAGE_ORDER_4K, q) )
+if ( p2m_pod_demand_populate(p2m, gfn_, PAGE_ORDER_4K, q) )
 goto out;
 }
 
diff --git a/xen/arch/x86/mm/p2m-pod.c b/xen/arch/x86/mm/p2m-pod.c
index 0dd0f0a083..5c79444d7b 100644
--- a/xen/arch/x86/mm/p2m-pod.c
+++ b/xen/arch/x86/mm/p2m-pod.c
@@ -1076,13 +1076,13 @@ static void pod_eager_record(struct p2m_domain *p2m, 
gfn_t gfn,
 }
 
 int
-p2m_pod_demand_populate(struct p2m_domain *p2m, unsigned long gfn,
+p2m_pod_demand_populate(struct p2m_domain *p2m, gfn_t gfn,
 unsigned int order,
 p2m_query_t q)
 {
 struct domain *d = p2m->domain;
 struct page_info *p = NULL; /* Compiler warnings */
-gfn_t gfn_aligned = _gfn((gfn >> order) << order);
+gfn_t gfn_aligned = _gfn((gfn_x(gfn) >> order) << order);
 mfn_t mfn;
 unsigned long i;
 
@@ -1135,8 +1135,8 @@ p2m_pod_demand_populate(struct p2m_domain *p2m, unsigned 
long gfn,
 goto out_of_memory;
 
 /* Keep track of the highest gfn demand-populated by a guest fault */
-if ( gfn > p2m->pod.max_guest )
-p2m->pod.max_guest = gfn;
+if ( gfn_x(gfn) > p2m->pod.max_guest )
+p2m->pod.max_guest = gfn_x(gfn);
 
 /*
  * Get a page f/ the cache.  A NULL return value indicates that the
@@ -1170,7 +1170,7 @@ p2m_pod_demand_populate(struct p2m_domain *p2m, unsigned 
long gfn,
 int d:16,order:16;
 } t;
 
-t.gfn = gfn;
+t.gfn = gfn_x(gfn);
 t.mfn = mfn_x(mfn);
 t.d = d->domain_id;
 t.order = order;
@@ -1210,7 +1210,7 @@ remap_and_retry:
 int d:16;
 } t;
 
-t.gfn = gfn;
+t.gfn = gfn_x(gfn);
 t.d = d->domain_id;
 
 __trace_var(TRC_MEM_POD_SUPERPAGE_SPLINTER, 0, sizeof(t), &t);
diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c
index 4bfec4f5f0..a639a00e9c 100644
--- a/xen/arch/x86/mm/p2m-pt.c
+++ b/xen/arch/x86/mm/p2m-pt.c
@@ -802,7 +802,7 @@ pod_retry_l3:
 {
 if ( q & P2M_ALLOC )
 {
-if ( !p2m_pod_demand_populate(p2m, gfn, PAGE_ORDER_1G, q) )
+if ( !p2m_pod_demand_populate(p2m, gfn_, PAGE_ORDER_1G, q) 
)
 goto pod_retry_l3;
 gdprintk(XENLOG_ERR, "%s: Allocate 1GB failed!\n", 
__func__);
 }
@@ -844,7 +844,7 @@ pod_retry_l2:
 if ( p2m_flags_to_type(flags) == p2m_populate_on_demand )
 {
 if ( q & P2M_ALLOC ) {
-if ( !p2m_pod_demand_populate(p2m, gfn, PAGE_ORDER_2M, q) )
+if ( !p2m_pod_demand_populate(p2m, gfn_, PAGE_ORDER_2M, q) )
 goto pod_retry_l2;
 } else
 *t = p2m_populate_on_demand;
@@ -883,7 +883,7 @@ pod_retry_l1:
 if ( l1t == p2m_populate_on_demand )
 {
 if ( q & P2M_ALLOC ) {
-if ( !p2m_pod_demand_populate(p2m, gfn, PAGE_ORDER_4K, q) )
+if ( !p2m_pod_demand_populate(p2m, gfn_, PAGE_ORDER_4K, q) )
 goto pod_retry_l1;
 } else
 *t = p2m_populate_on_demand;
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index 07ca02a173..1ae9216404 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -720,7 +720,7 @@ extern void audit_p2m(struct domain *d,
 
 /* Called by p2m code when demand-populating a PoD page */
 int
-p2m_pod_demand_populate(struct p2m_domain *p2m, unsigned long gfn,
+p2m_pod_demand_populate(struct p2m_domain *p2m, gfn_t gfn,
 unsigned int order,
 p2m_query_t q);
 
-- 
2.11.0


___
Xen-d

[Xen-devel] [PATCH v2 10/16] xen/x86: p2m-pod: Use typesafe GFN in pod_eager_record

[Julien Grall]

Signed-off-by: Julien Grall 
Acked-by: Andrew Cooper 

---

Cc: George Dunlap 
Cc: Jan Beulich 
Cc: Andrew Cooper 

Changes in v2:
- Add Andrew's acked-by
---
 xen/arch/x86/mm/p2m-pod.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/mm/p2m-pod.c b/xen/arch/x86/mm/p2m-pod.c
index b8a51cf12a..176d06cb42 100644
--- a/xen/arch/x86/mm/p2m-pod.c
+++ b/xen/arch/x86/mm/p2m-pod.c
@@ -1062,15 +1062,15 @@ static void pod_eager_reclaim(struct p2m_domain *p2m)
 } while ( (p2m->pod.count == 0) && (i < ARRAY_SIZE(mrp->list)) );
 }
 
-static void pod_eager_record(struct p2m_domain *p2m,
- unsigned long gfn, unsigned int order)
+static void pod_eager_record(struct p2m_domain *p2m, gfn_t gfn,
+ unsigned int order)
 {
 struct pod_mrp_list *mrp = &p2m->pod.mrp;
 
-ASSERT(gfn != gfn_x(INVALID_GFN));
+ASSERT(!gfn_eq(gfn, INVALID_GFN));
 
 mrp->list[mrp->idx++] =
-gfn | (order == PAGE_ORDER_2M ? POD_LAST_SUPERPAGE : 0);
+gfn_x(gfn) | (order == PAGE_ORDER_2M ? POD_LAST_SUPERPAGE : 0);
 mrp->idx %= ARRAY_SIZE(mrp->list);
 }
 
@@ -1160,7 +1160,7 @@ p2m_pod_demand_populate(struct p2m_domain *p2m, unsigned 
long gfn,
 p2m->pod.entry_count -= (1UL << order);
 BUG_ON(p2m->pod.entry_count < 0);
 
-pod_eager_record(p2m, gfn_x(gfn_aligned), order);
+pod_eager_record(p2m, gfn_aligned, order);
 
 if ( tb_init_done )
 {
-- 
2.11.0


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

[Xen-devel] [PATCH v2 00/16] xen/x86: Clean-up the PoD code

[Julien Grall]

Hi all,

I have been attempting to use the PoD code on Arm (it will be sent in a
separate series) and spent sometimes to clean-up and switch to typesafe gfn
the current code.

The PoD code has been tested on Arm (the vervision is slightly different,
mostly renaming) and the x86 part as only been built test it.

Cheers,

Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Jan Beulich 
Cc: Jun Nakajima 
Cc: Kevin Tian 
Cc: Razvan Cojocaru 
Cc: Tamas K Lengyel 

Julien Grall (16):
  xen/x86: p2m-pod: Clean-up includes
  xen/x86: p2m-pod: Remove trailing whitespaces
  xen/x86: p2m-pod: Fix coding style for comments
  xen/x86: p2m-pod: Fix coding style
  xen/x86: p2m-pod: Avoid redundant assignments in
p2m_pod_demand_populate
  xen/x86: p2m-pod: Clean-up use of typesafe MFN
  xen/x86: p2m-pod: Use typesafe gfn in p2m_pod_decrease_reservation
  xen/x86: p2m: Use typesafe gfn for the P2M callbacks get_entry and
set_entry
  xen/x86: p2m: Use typesafe GFN in p2m_set_entry
  xen/x86: p2m-pod: Use typesafe GFN in pod_eager_record
  xen/x86: p2m-pod: Clean-up p2m_pod_zero_check
  xen/x86: p2m-pod: Use typesafe gfn in p2m_pod_zero_check
  xen/x86: p2m-pod: Use typesafe gfn in p2m_pod_demand_populate
  xen/x86: p2m-pod: Use typesafe gfn for the fields reclaim_single and
max_guest
  xen/x86: p2m-pod: Rework prototype of p2m_pod_demand_populate
  xen/x86: mem_access: Fix mis-indented line

 xen/arch/arm/p2m.c   |   3 +-
 xen/arch/x86/hvm/hvm.c   |   2 +-
 xen/arch/x86/mm/hap/nested_hap.c |   2 +-
 xen/arch/x86/mm/mem_access.c |  21 +-
 xen/arch/x86/mm/mem_sharing.c|   7 +-
 xen/arch/x86/mm/p2m-ept.c|  11 +-
 xen/arch/x86/mm/p2m-pod.c| 435 +--
 xen/arch/x86/mm/p2m-pt.c |  12 +-
 xen/arch/x86/mm/p2m.c| 139 +++--
 xen/common/memory.c  |   3 +-
 xen/include/asm-arm/p2m.h|  13 --
 xen/include/asm-x86/p2m.h|  23 +--
 xen/include/xen/p2m-common.h |  13 ++
 13 files changed, 371 insertions(+), 313 deletions(-)

-- 
2.11.0


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

[Xen-devel] [PATCH v2 12/16] xen/x86: p2m-pod: Use typesafe gfn in p2m_pod_zero_check

[Julien Grall]

At the same time make the array gfns const has it is not modified within
the function.

Signed-off-by: Julien Grall 
Acked-by: Andrew Cooper 

---

Cc: George Dunlap 
Cc: Jan Beulich 
Cc: Andrew Cooper 

Changes in v2:
- Add Andrew's acked-by
---
 xen/arch/x86/mm/p2m-pod.c | 37 ++---
 1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/xen/arch/x86/mm/p2m-pod.c b/xen/arch/x86/mm/p2m-pod.c
index 611a087855..0dd0f0a083 100644
--- a/xen/arch/x86/mm/p2m-pod.c
+++ b/xen/arch/x86/mm/p2m-pod.c
@@ -498,7 +498,7 @@ p2m_pod_offline_or_broken_replace(struct page_info *p)
 }
 
 static int
-p2m_pod_zero_check_superpage(struct p2m_domain *p2m, unsigned long gfn);
+p2m_pod_zero_check_superpage(struct p2m_domain *p2m, gfn_t gfn);
 
 
 /*
@@ -582,7 +582,7 @@ p2m_pod_decrease_reservation(struct domain *d, gfn_t gfn, 
unsigned int order)
  * - not all of the pages were RAM (now knowing order < SUPERPAGE_ORDER)
  */
 if ( steal_for_cache && order < SUPERPAGE_ORDER && ram == (1UL << order) &&
- p2m_pod_zero_check_superpage(p2m, gfn_x(gfn) & ~(SUPERPAGE_PAGES - 
1)) )
+ p2m_pod_zero_check_superpage(p2m, _gfn(gfn_x(gfn) & ~(SUPERPAGE_PAGES 
- 1))) )
 {
 pod = 1UL << order;
 ram = nonpod = 0;
@@ -680,10 +680,9 @@ void p2m_pod_dump_data(struct domain *d)
  * in the p2m.
  */
 static int
-p2m_pod_zero_check_superpage(struct p2m_domain *p2m, unsigned long gfn_l)
+p2m_pod_zero_check_superpage(struct p2m_domain *p2m, gfn_t gfn)
 {
 mfn_t mfn, mfn0 = INVALID_MFN;
-gfn_t gfn = _gfn(gfn_l);
 p2m_type_t type, type0 = 0;
 unsigned long * map = NULL;
 int ret=0, reset = 0;
@@ -694,7 +693,7 @@ p2m_pod_zero_check_superpage(struct p2m_domain *p2m, 
unsigned long gfn_l)
 
 ASSERT(pod_locked_by_me(p2m));
 
-if ( !superpage_aligned(gfn_l) )
+if ( !superpage_aligned(gfn_x(gfn)) )
 goto out;
 
 /* Allow an extra refcount for one shadow pt mapping in shadowed domains */
@@ -816,7 +815,7 @@ p2m_pod_zero_check_superpage(struct p2m_domain *p2m, 
unsigned long gfn_l)
 int d:16,order:16;
 } t;
 
-t.gfn = gfn_l;
+t.gfn = gfn_x(gfn);
 t.mfn = mfn_x(mfn);
 t.d = d->domain_id;
 t.order = 9;
@@ -843,7 +842,7 @@ out:
 }
 
 static void
-p2m_pod_zero_check(struct p2m_domain *p2m, unsigned long *gfns, int count)
+p2m_pod_zero_check(struct p2m_domain *p2m, const gfn_t *gfns, int count)
 {
 mfn_t mfns[count];
 p2m_type_t types[count];
@@ -863,7 +862,7 @@ p2m_pod_zero_check(struct p2m_domain *p2m, unsigned long 
*gfns, int count)
 p2m_access_t a;
 struct page_info *pg;
 
-mfns[i] = p2m->get_entry(p2m, _gfn(gfns[i]), types + i, &a,
+mfns[i] = p2m->get_entry(p2m, gfns[i], types + i, &a,
  0, NULL, NULL);
 pg = mfn_to_page(mfns[i]);
 
@@ -901,7 +900,7 @@ p2m_pod_zero_check(struct p2m_domain *p2m, unsigned long 
*gfns, int count)
 }
 
 /* Try to remove the page, restoring old mapping if it fails. */
-p2m_set_entry(p2m, _gfn(gfns[i]), INVALID_MFN, PAGE_ORDER_4K,
+p2m_set_entry(p2m, gfns[i], INVALID_MFN, PAGE_ORDER_4K,
   p2m_populate_on_demand, p2m->default_access);
 
 /*
@@ -913,7 +912,7 @@ p2m_pod_zero_check(struct p2m_domain *p2m, unsigned long 
*gfns, int count)
 unmap_domain_page(map[i]);
 map[i] = NULL;
 
-p2m_set_entry(p2m, _gfn(gfns[i]), mfns[i], PAGE_ORDER_4K,
+p2m_set_entry(p2m, gfns[i], mfns[i], PAGE_ORDER_4K,
 types[i], p2m->default_access);
 
 continue;
@@ -940,7 +939,7 @@ p2m_pod_zero_check(struct p2m_domain *p2m, unsigned long 
*gfns, int count)
  */
 if ( j < (PAGE_SIZE / sizeof(*map[i])) )
 {
-p2m_set_entry(p2m, _gfn(gfns[i]), mfns[i], PAGE_ORDER_4K,
+p2m_set_entry(p2m, gfns[i], mfns[i], PAGE_ORDER_4K,
   types[i], p2m->default_access);
 }
 else
@@ -952,7 +951,7 @@ p2m_pod_zero_check(struct p2m_domain *p2m, unsigned long 
*gfns, int count)
 int d:16,order:16;
 } t;
 
-t.gfn = gfns[i];
+t.gfn = gfn_x(gfns[i]);
 t.mfn = mfn_x(mfns[i]);
 t.d = d->domain_id;
 t.order = 0;
@@ -973,7 +972,7 @@ p2m_pod_zero_check(struct p2m_domain *p2m, unsigned long 
*gfns, int count)
 static void
 p2m_pod_emergency_sweep(struct p2m_domain *p2m)
 {
-unsigned long gfns[POD_SWEEP_STRIDE];
+gfn_t gfns[POD_SWEEP_STRIDE];
 unsigned long i, j = 0, start, limit;
 p2m_type_t t;
 
@@ -997,7 +996,7 @@ p2m_pod_emergency_sweep(struct p2m_domain *p2m)
 (void)p2m->get_entry(p2m, _gfn(i), &t, &a, 0, NULL, NULL);
 if ( p2m_is_ram(t) )
 {
-gfns[j] = i;
+gfns[j] = _gfn(i);
 j++;
 BUG_ON(j > P

[Xen-devel] [PATCH v2 16/16] xen/x86: mem_access: Fix mis-indented line

[Julien Grall]

Signed-off-by: Julien Grall 

---

Cc: Razvan Cojocaru 
Cc: Tamas K Lengyel 
Cc: George Dunlap 
Cc: Jan Beulich 
Cc: Andrew Cooper 

Changes in v2:
- Patch added
---
 xen/arch/x86/mm/mem_access.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/mm/mem_access.c b/xen/arch/x86/mm/mem_access.c
index 948e377e69..5c1008890e 100644
--- a/xen/arch/x86/mm/mem_access.c
+++ b/xen/arch/x86/mm/mem_access.c
@@ -274,7 +274,7 @@ int p2m_set_altp2m_mem_access(struct domain *d, struct 
p2m_domain *hp2m,
 }
 
 return ap2m->set_entry(ap2m, gfn, mfn, PAGE_ORDER_4K, t, a,
- (current->domain != d));
+   (current->domain != d));
 }
 
 static int set_mem_access(struct domain *d, struct p2m_domain *p2m,
-- 
2.11.0


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

Re: [Xen-devel] [PATCH v12 1/4] x86emul: New return code for unimplemented instruction

[Jan Beulich]

>>> On 21.09.17 at 07:12,  wrote:
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -6195,7 +6196,7 @@ x86_emulate(
>  /* vpsll{w,d} $imm8,{x,y}mm,{x,y}mm */
>  break;
>  default:
> -goto cannot_emulate;
> +goto unimplemented_insn;

I would really appreciate if you were a little more patient and waited
for replies to earlier review threads before sending a new version.
As said on the v11 thread, this ought to be "unrecognized".

> @@ -6243,7 +6244,8 @@ x86_emulate(
>  case 6: /* psllq $imm8,mm */
>  goto simd_0f_shift_imm;
>  }
> -goto cannot_emulate;
> +rc = X86EMUL_UNRECOGNIZED;
> +goto done;

I think it would read better if we had an "unrecognized_insn"
label just like now we gain an "unimplemented_insn" one. Whether
the _insn suffixes are really useful is another question.

> --- a/xen/arch/x86/x86_emulate/x86_emulate.h
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.h
> @@ -133,6 +133,19 @@ struct x86_emul_fpu_aux {
>* Undefined behavior when used anywhere else.
>*/
>  #define X86EMUL_DONE   4
> + /*
> +  * Current instruction is not implemented by the emulator.
> +  * This value should only be returned by the core emulator when a valid
> +  * opcode is found but the execution logic for that instruction is missing.
> +  * It should NOT be returned by any of the x86_emulate_ops callbacks.
> +  */
> +#define X86EMUL_UNIMPLEMENTED  5
> + /*
> +  * The current instruction's opcode is not valid.
> +  * If this error code is returned by a function, an #UD trap should be
> +  * raised by the final consumer of it.
> + */
> +#define X86EMUL_UNRECOGNIZED   X86EMUL_UNIMPLEMENTED

But with this aliasing of values the comment still is somewhat off.

Jan


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

Re: [Xen-devel] [PATCH v12 4/4] x86emul: Raise #UD when emulating an unrecognized instruction.

[Jan Beulich]

>>> On 21.09.17 at 10:57,  wrote:
>> From: Petre Pircalabu [mailto:ppircal...@bitdefender.com]
>> Sent: 21 September 2017 06:12
>> --- a/xen/arch/x86/hvm/vmx/realmode.c
>> +++ b/xen/arch/x86/hvm/vmx/realmode.c
>> @@ -106,12 +106,21 @@ void vmx_realmode_emulate_one(struct
>> hvm_emulate_ctxt *hvmemul_ctxt)
>>  if ( hvm_vcpu_io_need_completion(vio) || vio->mmio_retry )
>>  vio->io_completion = HVMIO_realmode_completion;
>> 
>> -if ( rc == X86EMUL_UNHANDLEABLE || rc == X86EMUL_UNIMPLEMENTED
>> )
>> +if ( rc == X86EMUL_UNHANDLEABLE )
> 
> I don't quite understand this change. Why has it become unnecessary to deal 
> with X86EMUL_UNIMPLEMENTED? Patch #1 added this change so it seems odd that 
> patch #4 would then revert it.

Yeah, it would certainly be more natural to bring things into
their final shape right away.

Jan


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

[Xen-devel] [PATCH v2 04/16] xen/x86: p2m-pod: Fix coding style

[Julien Grall]

Also take the opportunity to:
- move from 1 << * to 1UL << *.
- use unsigned when possible
- move from unsigned int -> unsigned long for some induction
variables

Signed-off-by: Julien Grall 
Acked-by: Andrew Cooper 

---

Cc: George Dunlap 
Cc: Jan Beulich 
Cc: Andrew Cooper 

Changes in v2:
- Add Andrew's acked-by
---
 xen/arch/x86/mm/p2m-pod.c | 102 +++---
 1 file changed, 52 insertions(+), 50 deletions(-)

diff --git a/xen/arch/x86/mm/p2m-pod.c b/xen/arch/x86/mm/p2m-pod.c
index 6f045081b5..f04d6e03e2 100644
--- a/xen/arch/x86/mm/p2m-pod.c
+++ b/xen/arch/x86/mm/p2m-pod.c
@@ -60,7 +60,7 @@ p2m_pod_cache_add(struct p2m_domain *p2m,
   struct page_info *page,
   unsigned int order)
 {
-int i;
+unsigned long i;
 struct page_info *p;
 struct domain *d = p2m->domain;
 
@@ -70,23 +70,24 @@ p2m_pod_cache_add(struct p2m_domain *p2m,
 mfn = page_to_mfn(page);
 
 /* Check to make sure this is a contiguous region */
-if( mfn_x(mfn) & ((1 << order) - 1) )
+if ( mfn_x(mfn) & ((1UL << order) - 1) )
 {
 printk("%s: mfn %lx not aligned order %u! (mask %lx)\n",
__func__, mfn_x(mfn), order, ((1UL << order) - 1));
 return -1;
 }
 
-for(i=0; i < 1 << order ; i++) {
+for ( i = 0; i < 1UL << order ; i++)
+{
 struct domain * od;
 
 p = mfn_to_page(_mfn(mfn_x(mfn) + i));
 od = page_get_owner(p);
-if(od != d)
+if ( od != d )
 {
 printk("%s: mfn %lx expected owner d%d, got owner d%d!\n",
__func__, mfn_x(mfn), d->domain_id,
-   od?od->domain_id:-1);
+   od ? od->domain_id : -1);
 return -1;
 }
 }
@@ -99,12 +100,12 @@ p2m_pod_cache_add(struct p2m_domain *p2m,
  * guaranteed to be zero; but by reclaiming zero pages, we implicitly
  * promise to provide zero pages. So we scrub pages before using.
  */
-for ( i = 0; i < (1 << order); i++ )
+for ( i = 0; i < (1UL << order); i++ )
 clear_domain_page(_mfn(mfn_x(page_to_mfn(page)) + i));
 
 /* First, take all pages off the domain list */
 lock_page_alloc(p2m);
-for(i=0; i < 1 << order ; i++)
+for ( i = 0; i < 1UL << order ; i++ )
 {
 p = page + i;
 page_list_del(p, &d->page_list);
@@ -128,7 +129,7 @@ p2m_pod_cache_add(struct p2m_domain *p2m,
 default:
 BUG();
 }
-p2m->pod.count += 1L << order;
+p2m->pod.count += 1UL << order;
 
 return 0;
 }
@@ -140,7 +141,7 @@ static struct page_info * p2m_pod_cache_get(struct 
p2m_domain *p2m,
 unsigned int order)
 {
 struct page_info *p = NULL;
-int i;
+unsigned long i;
 
 ASSERT(pod_locked_by_me(p2m));
 
@@ -162,7 +163,7 @@ static struct page_info * p2m_pod_cache_get(struct 
p2m_domain *p2m,
 p = page_list_remove_head(&p2m->pod.super);
 mfn = mfn_x(page_to_mfn(p));
 
-for ( i=0; ipod.single);
@@ -174,12 +175,12 @@ static struct page_info * p2m_pod_cache_get(struct 
p2m_domain *p2m,
 case PAGE_ORDER_2M:
 BUG_ON( page_list_empty(&p2m->pod.super) );
 p = page_list_remove_head(&p2m->pod.super);
-p2m->pod.count -= 1 << order;
+p2m->pod.count -= 1UL << order;
 break;
 case PAGE_ORDER_4K:
 BUG_ON( page_list_empty(&p2m->pod.single) );
 p = page_list_remove_head(&p2m->pod.single);
-p2m->pod.count -= 1;
+p2m->pod.count -= 1UL;
 break;
 default:
 BUG();
@@ -187,7 +188,7 @@ static struct page_info * p2m_pod_cache_get(struct 
p2m_domain *p2m,
 
 /* Put the pages back on the domain page_list */
 lock_page_alloc(p2m);
-for ( i = 0 ; i < (1 << order); i++ )
+for ( i = 0 ; i < (1UL << order); i++ )
 {
 BUG_ON(page_get_owner(p + i) != p2m->domain);
 page_list_add_tail(p + i, &p2m->domain->page_list);
@@ -251,7 +252,8 @@ p2m_pod_set_cache_target(struct p2m_domain *p2m, unsigned 
long pod_target, int p
 while ( pod_target < p2m->pod.count )
 {
 struct page_info * page;
-int order, i;
+unsigned int order;
+unsigned long i;
 
 if ( (p2m->pod.count - pod_target) > SUPERPAGE_PAGES
  && !page_list_empty(&p2m->pod.super) )
@@ -264,10 +266,10 @@ p2m_pod_set_cache_target(struct p2m_domain *p2m, unsigned 
long pod_target, int p
 ASSERT(page != NULL);
 
 /* Then free them */
-for ( i = 0 ; i < (1 << order) ; i++ )
+for ( i = 0 ; i < (1UL << order) ; i++ )
 {
 /* Copied from common/memory.c:guest_remove_page() */
-if ( unlikely(!get_page(page+i, d)) )
+if ( unlikely(!get_page(page + i, d)) )
 {
 gdprintk(XENLOG_INFO, "Bad page free for domain %u\n", 
d->domain_id);
 ret = -EINVAL;
@@ -

[Xen-devel] [PATCH v2 03/16] xen/x86: p2m-pod: Fix coding style for comments

[Julien Grall]

Signed-off-by: Julien Grall 
Acked-by: Andrew Cooper 

---

Cc: George Dunlap 
Cc: Jan Beulich 
Cc: Andrew Cooper 

Changes in v2:
- Add Andrew's acked-by
---
 xen/arch/x86/mm/p2m-pod.c | 154 ++
 1 file changed, 102 insertions(+), 52 deletions(-)

diff --git a/xen/arch/x86/mm/p2m-pod.c b/xen/arch/x86/mm/p2m-pod.c
index 1f07441259..6f045081b5 100644
--- a/xen/arch/x86/mm/p2m-pod.c
+++ b/xen/arch/x86/mm/p2m-pod.c
@@ -155,8 +155,10 @@ static struct page_info * p2m_pod_cache_get(struct 
p2m_domain *p2m,
 
 BUG_ON( page_list_empty(&p2m->pod.super) );
 
-/* Break up a superpage to make single pages. NB count doesn't
- * need to be adjusted. */
+/*
+ * Break up a superpage to make single pages. NB count doesn't
+ * need to be adjusted.
+ */
 p = page_list_remove_head(&p2m->pod.super);
 mfn = mfn_x(page_to_mfn(p));
 
@@ -242,8 +244,10 @@ p2m_pod_set_cache_target(struct p2m_domain *p2m, unsigned 
long pod_target, int p
 }
 
 /* Decreasing the target */
-/* We hold the pod lock here, so we don't need to worry about
- * cache disappearing under our feet. */
+/*
+ * We hold the pod lock here, so we don't need to worry about
+ * cache disappearing under our feet.
+ */
 while ( pod_target < p2m->pod.count )
 {
 struct page_info * page;
@@ -345,15 +349,19 @@ p2m_pod_set_mem_target(struct domain *d, unsigned long 
target)
 if ( d->is_dying )
 goto out;
 
-/* T' < B: Don't reduce the cache size; let the balloon driver
- * take care of it. */
+/*
+ * T' < B: Don't reduce the cache size; let the balloon driver
+ * take care of it.
+ */
 if ( target < d->tot_pages )
 goto out;
 
 pod_target = target - populated;
 
-/* B < T': Set the cache size equal to # of outstanding entries,
- * let the balloon driver fill in the rest. */
+/*
+ * B < T': Set the cache size equal to # of outstanding entries,
+ * let the balloon driver fill in the rest.
+ */
 if ( populated > 0 && pod_target > p2m->pod.entry_count )
 pod_target = p2m->pod.entry_count;
 
@@ -491,7 +499,8 @@ static int
 p2m_pod_zero_check_superpage(struct p2m_domain *p2m, unsigned long gfn);
 
 
-/* This function is needed for two reasons:
+/*
+ * This function is needed for two reasons:
  * + To properly handle clearing of PoD entries
  * + To "steal back" memory being freed for the PoD cache, rather than
  *   releasing it.
@@ -513,8 +522,10 @@ p2m_pod_decrease_reservation(struct domain *d,
 gfn_lock(p2m, gpfn, order);
 pod_lock(p2m);
 
-/* If we don't have any outstanding PoD entries, let things take their
- * course */
+/*
+ * If we don't have any outstanding PoD entries, let things take their
+ * course.
+ */
 if ( p2m->pod.entry_count == 0 )
 goto out_unlock;
 
@@ -550,8 +561,10 @@ p2m_pod_decrease_reservation(struct domain *d,
 
 if ( !nonpod )
 {
-/* All PoD: Mark the whole region invalid and tell caller
- * we're done. */
+/*
+ * All PoD: Mark the whole region invalid and tell caller
+ * we're done.
+ */
 p2m_set_entry(p2m, gpfn, INVALID_MFN, order, p2m_invalid,
   p2m->default_access);
 p2m->pod.entry_count-=(1<= SUPERPAGE_ORDER (the loop below will take care of this)
  * - not all of the pages were RAM (now knowing order < SUPERPAGE_ORDER)
  */
-if ( steal_for_cache && order < SUPERPAGE_ORDER && ram == (1 << order) &&
+if ( steal_for_cache && order < SUPERPAGE_ORDER && ram == (1UL << order) &&
  p2m_pod_zero_check_superpage(p2m, gpfn & ~(SUPERPAGE_PAGES - 1)) )
 {
-pod = 1 << order;
+pod = 1UL << order;
 ram = nonpod = 0;
 ASSERT(steal_for_cache == (p2m->pod.entry_count > p2m->pod.count));
 }
 
-/* Process as long as:
+/*
+ * Process as long as:
  * + There are PoD entries to handle, or
  * + There is ram left, and we want to steal it
  */
@@ -631,8 +645,10 @@ p2m_pod_decrease_reservation(struct domain *d,
 }
 }
 
-/* If there are no more non-PoD entries, tell decrease_reservation() that
- * there's nothing left to do. */
+/*
+ * If there are no more non-PoD entries, tell decrease_reservation() that
+ * there's nothing left to do.
+ */
 if ( nonpod == 0 )
 ret = 1;
 
@@ -658,9 +674,11 @@ void p2m_pod_dump_data(struct domain *d)
 }
 
 
-/* Search for all-zero superpages to be reclaimed as superpages for the
+/*
+ * Search for all-zero superpages to be reclaimed as superpages for the
  * PoD cache. Must be called w/ pod lock held, must lock the superpage
- * in the p2m */
+ * in the p2m.
+ */
 static int
 p2m_pod_zero_check_superpage(struct p2m_domain *p2m, unsigned long gfn)
 {
@@ -682,12 +700,16 @@ p2m_pod_zero_check_superpage(struct p2m_d

[Xen-devel] [xen-unstable-smoke test] 113662: tolerable all pass - PUSHED

[osstest service owner]

flight 113662 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/113662/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  00cc5cd669122b77b336dae52566e813ec45122d
baseline version:
 xen  0b2cadaaf2bc5216c2a6e43ada24c965380bf094

Last test of basis   113643  2017-09-20 22:12:59 Z0 days
Testing same since   113662  2017-09-21 11:02:13 Z0 days1 attempts


People who touched revisions under test:
  Dario Faggioli 

jobs:
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-amd64-amd64-xl-qemuu-debianhvm-i386 pass
 test-amd64-amd64-libvirt pass



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


Pushing revision :

+ branch=xen-unstable-smoke
+ revision=00cc5cd669122b77b336dae52566e813ec45122d
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
 export PERLLIB=.:.
 PERLLIB=.:.
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x '!=' x/home/osstest/repos/lock ']'
++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock
++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push xen-unstable-smoke 
00cc5cd669122b77b336dae52566e813ec45122d
+ branch=xen-unstable-smoke
+ revision=00cc5cd669122b77b336dae52566e813ec45122d
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
 export PERLLIB=.:.:.
 PERLLIB=.:.:.
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']'
+ . ./cri-common
++ . ./cri-getconfig
+++ export PERLLIB=.:.:.:.
+++ PERLLIB=.:.:.:.
++ umask 002
+ select_xenbranch
+ case "$branch" in
+ tree=xen
+ xenbranch=xen-unstable-smoke
+ qemuubranch=qemu-upstream-unstable
+ '[' xxen = xlinux ']'
+ linuxbranch=
+ '[' xqemu-upstream-unstable = x ']'
+ select_prevxenbranch
++ ./cri-getprevxenbranch xen-unstable-smoke
+ prevxenbranch=xen-4.9-testing
+ '[' x00cc5cd669122b77b336dae52566e813ec45122d = x ']'
+ : tested/2.6.39.x
+ . ./ap-common
++ : osst...@xenbits.xen.org
+++ getconfig OsstestUpstream
+++ perl -e '
use Osstest;
readglobalconfig();
print $c{"OsstestUpstream"} or die $!;
'
++ :
++ : git://xenbits.xen.org/xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/xen.git
++ : git://xenbits.xen.org/qemu-xen-traditional.git
++ : git://git.kernel.org
++ : git://git.kernel.org/pub/scm/linux/kernel/git
++ : git
++ : git://xenbits.xen.org/xtf.git
++ : osst...@xenbits.xen.org:/home/xen/git/xtf.git
++ : git://xenbits.xen.org/xtf.git
++ : git://xenbits.xen.org/libvirt.git
++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git
++ : git://xenbits.xen.org/libvirt.git
++ : git://xenbits.xen.org/osstest/rumprun.git
++ : git
++ : git://xenbits.xen.org/osstest/rumprun.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/rumprun.git
++ : git://git.seabios.org/seabios.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/seabios.git
++ : git://xenbits.xen.org/osstest/seabios.git
++ : https://github.com/tianocore/edk2.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/ovmf.git
++ : git://xenbits.xen.org/osstest/ovmf.git
++ : git://xenbits.xen.org/osstest/linux-fir

Re: [Xen-devel] [PATCH v2 16/16] xen/x86: mem_access: Fix mis-indented line

[Razvan Cojocaru]

On 09/21/2017 03:40 PM, Julien Grall wrote:
> Signed-off-by: Julien Grall 
> 
> ---
> 
> Cc: Razvan Cojocaru 
> Cc: Tamas K Lengyel 
> Cc: George Dunlap 
> Cc: Jan Beulich 
> Cc: Andrew Cooper 

Acked-by: Razvan Cojocaru 


Thanks,
Razvan

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

Re: [Xen-devel] [PATCH v1] x86: fix bug caused by commit 0ade5e

[Jan Beulich]

>>> On 21.09.17 at 10:11,  wrote:

A better title for the patch is needed.

> Commit 0ade5e causes a bug that only the psr features presented in cmdline
> cannot be correctly enumerated.

To me it looks like either the "only" is wrong here, or the "cannot"
was meant to be "can"; the way it's now I can't make sense of it.

> 1. If there is only 'psr=', the CMT is enumerated which is not right.

s/,the /,then/ ?

Jan


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

Re: [Xen-devel] [PATCH] xen/perfc: Fix build after commit fc32575968 when CONFIG_PERF_COUNTERS=y

[Jan Beulich]

>>> On 21.09.17 at 13:35,  wrote:
> The commit fc32575968 "public/sysctl: drop unnecessary typedefs and
> handles" went a bit too far by replacing all xen_systcl_*_t type to
> struct xen_sysctl_*.
> 
> However, xen_sysctl_perfc_val_t was a typedef on uint32_t and therefore
> is not associated to a structure.
> 
> Use xen_sysctl_perfc_val_t to fix the build when CONFIG_PERF_COUNTERS=y
> 
> Signed-off-by: Julien Grall 

Oops, I'm sorry.
Acked-by: Jan Beulich 

Jan


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

Re: [Xen-devel] [PATCH v2 16/16] xen/x86: mem_access: Fix mis-indented line

[Jan Beulich]

>>> On 21.09.17 at 14:40,  wrote:
> --- a/xen/arch/x86/mm/mem_access.c
> +++ b/xen/arch/x86/mm/mem_access.c
> @@ -274,7 +274,7 @@ int p2m_set_altp2m_mem_access(struct domain *d, struct 
> p2m_domain *hp2m,
>  }
>  
>  return ap2m->set_entry(ap2m, gfn, mfn, PAGE_ORDER_4K, t, a,
> - (current->domain != d));
> +   (current->domain != d));

But once touching it, the stray parentheses could go away as well.

Jan


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

Re: [Xen-devel] [PATCH 1/2] common/efi: bail if dom0 fails the shim verification step

[Jan Beulich]

>>> On 20.09.17 at 22:57,  wrote:
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -1226,9 +1226,13 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE 
> *SystemTable)
>  efi_bs->FreePool(name.w);
>  
>  if ( !EFI_ERROR(efi_bs->LocateProtocol(&shim_lock_guid, NULL,
> -(void **)&shim_lock)) &&
> - (status = shim_lock->Verify(kernel.ptr, kernel.size)) != 
> EFI_SUCCESS )
> -PrintErrMesg(L"Dom0 kernel image could not be verified", status);
> +(void **)&shim_lock)))
> +{
> +if  ( shim_lock->Verify(kernel.ptr, kernel.size) != EFI_SUCCESS )
> +blexit(L"Dom0 kernel image could not be verified by the 
> shim.");
> +
> +PrintStr(L"Dom0 kernel image was verified by the shim.\r\n");
> +}

So what is the actual behavioral change you're trying to
accomplish? PrintErrMesg() already calls blexit(), and I hope
sure the purpose of the change is neither to open code
anything, nor to drop the printing of the error code. And I
don't see any value in the success case message - it'll be
visible for a very brief moment at best anyway.

Jan


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

Re: [Xen-devel] [PATCH 2/2] common/efi: give people some time to read messages when debugging

[Jan Beulich]

>>> On 20.09.17 at 22:57,  wrote:
> From: Tamas K Lengyel 
> 
> The EFI messages flash by so fast that it is impossible to catch them without
> a serial debugger attached. Sometimes though we don't have that available so
> having some time to read the messages off the screen is valuable.
> 
> Signed-off-by: Tamas K Lengyel 

NAK: I don't want any unnecessary stalls, including on debug
builds. If you want such stalls for yourself, patch then in as
needed.

Jan


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

Re: [Xen-devel] pci-passthrough loses msi-x interrupts ability after domain destroy

[Jan Beulich]

>>> On 20.09.17 at 21:50,  wrote:
> - On Dom0, 'lspci -vv' on that PCIe device between the "working" and 
> the "muted interrupts" states, I noted a difference between the 
> MSI-X caps: 
> 
> - Capabilities: [70] MSI-X: Enable- Count=5 Masked- <-- IRQs will work if 
> domain started 
> + Capabilities: [70] MSI-X: Enable- Count=5 Masked+ <-- IRQs won't work if 
> domain started
> ^^^

And did you verify that the OS actually makes an attempt to clear
this mask-all flag? If such an attempt doesn't have the intended
effect, finding the problem location in the code and sending a
fix can't be that difficult. If otoh the guest doesn't do this, then
we'd need to figure out whether we leave the device in a wrong
state after de-assigning it from the original guest instance (albeit,
as Roger said, the reset the device is supposed to go through
would be expected to clear it). I can certainly see an OS not
necessarily expecting the bit to be set when first gaining control
of the device. For this, look at the lspci output for the device in
Dom0 between shutting down and then restarting the guest.

Jan


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

[Xen-devel] [PATCH v4 3/5] ARM: ITS: Deny hardware domain access to ITS

[mjaggi]

From: Manish Jaggi 

This patch extends the gicv3_iomem_deny_access functionality by adding
support for ITS region as well. Add function gicv3_its_deny_access.

Signed-off-by: Manish Jaggi 
---
 xen/arch/arm/gic-v3-its.c| 22 ++
 xen/arch/arm/gic-v3.c|  3 +++
 xen/include/asm-arm/gic_v3_its.h |  9 +
 3 files changed, 34 insertions(+)

diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
index 0f662cf..8697e5b 100644
--- a/xen/arch/arm/gic-v3-its.c
+++ b/xen/arch/arm/gic-v3-its.c
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -905,6 +906,27 @@ struct pending_irq *gicv3_assign_guest_event(struct domain 
*d,
 return pirq;
 }
 
+int gicv3_its_deny_access(const struct domain *d)
+{
+int rc = 0;
+unsigned long mfn, nr;
+const struct host_its *its_data;
+
+list_for_each_entry( its_data, &host_its_list, entry )
+{
+mfn = paddr_to_pfn(its_data->addr);
+nr = PFN_UP(GICV3_ITS_SIZE);
+rc = iomem_deny_access(d, mfn, mfn + nr);
+if ( rc )
+{
+printk( "iomem_deny_access failed for %lx:%lx \r\n", mfn, nr);
+break;
+}
+}
+
+return rc;
+}
+
 /*
  * Create the respective guest DT nodes from a list of host ITSes.
  * This copies the reg property, so the guest sees the ITS at the same address
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 6f562f4..b3d605d 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1308,6 +1308,9 @@ static int gicv3_iomem_deny_access(const struct domain *d)
 if ( rc )
 return rc;
 
+if ( gicv3_its_deny_access(d) )
+return rc;
+
 for ( i = 0; i < gicv3.rdist_count; i++ )
 {
 mfn = gicv3.rdist_regions[i].base >> PAGE_SHIFT;
diff --git a/xen/include/asm-arm/gic_v3_its.h b/xen/include/asm-arm/gic_v3_its.h
index e1be33c..31fca66 100644
--- a/xen/include/asm-arm/gic_v3_its.h
+++ b/xen/include/asm-arm/gic_v3_its.h
@@ -139,6 +139,10 @@ void gicv3_its_dt_init(const struct dt_device_node *node);
 #ifdef CONFIG_ACPI
 void gicv3_its_acpi_init(void);
 #endif
+
+/* Deny iomem access for its */
+int gicv3_its_deny_access(const struct domain *d);
+
 bool gicv3_its_host_has_its(void);
 
 unsigned int vgic_v3_its_count(const struct domain *d);
@@ -206,6 +210,11 @@ static inline void gicv3_its_acpi_init(void)
 }
 #endif
 
+static inline int gicv3_its_deny_access(const struct domain *d)
+{
+return 0;
+}
+
 static inline bool gicv3_its_host_has_its(void)
 {
 return false;
-- 
2.7.4


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

[Xen-devel] [PATCH v4 1/5] ARM: ITS: Introduce common function add_to_host_its_list

[mjaggi]

From: Manish Jaggi 

add_to_host_its_list will update the host_its_list. This common
function to be invoked from gicv3_its_dt_init and gic_v3_its_acpi_probe.

Signed-off-by: Manish Jaggi 
Reviewed-by: Andre Przywara 
Acked-by: Julien Grall 
---
 xen/arch/arm/gic-v3-its.c | 32 
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
index 2d36030..0610991 100644
--- a/xen/arch/arm/gic-v3-its.c
+++ b/xen/arch/arm/gic-v3-its.c
@@ -976,11 +976,29 @@ int gicv3_its_make_hwdom_dt_nodes(const struct domain *d,
 return res;
 }
 
+/* Common function for adding to host_its_list */
+static void add_to_host_its_list(paddr_t addr, paddr_t size,
+ const struct dt_device_node *node)
+{
+struct host_its *its_data;
+
+its_data = xzalloc(struct host_its);
+if ( !its_data )
+panic("GICv3: Cannot allocate memory for ITS frame");
+
+its_data->addr = addr;
+its_data->size = size;
+its_data->dt_node = node;
+
+printk("GICv3: Found ITS @0x%lx\n", addr);
+
+list_add_tail(&its_data->entry, &host_its_list);
+}
+
 /* Scan the DT for any ITS nodes and create a list of host ITSes out of it. */
 void gicv3_its_dt_init(const struct dt_device_node *node)
 {
 const struct dt_device_node *its = NULL;
-struct host_its *its_data;
 
 /*
  * Check for ITS MSI subnodes. If any, add the ITS register
@@ -996,17 +1014,7 @@ void gicv3_its_dt_init(const struct dt_device_node *node)
 if ( dt_device_get_address(its, 0, &addr, &size) )
 panic("GICv3: Cannot find a valid ITS frame address");
 
-its_data = xzalloc(struct host_its);
-if ( !its_data )
-panic("GICv3: Cannot allocate memory for ITS frame");
-
-its_data->addr = addr;
-its_data->size = size;
-its_data->dt_node = its;
-
-printk("GICv3: Found ITS @0x%lx\n", addr);
-
-list_add_tail(&its_data->entry, &host_its_list);
+add_to_host_its_list(addr, size, its);
 }
 }
 
-- 
2.7.4


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

[Xen-devel] [PATCH v4 0/5] ARM: ACPI: ITS: Add ITS Support for ACPI hardware domain

[mjaggi]

From: Manish Jaggi 

This patch is split into 5 patches. First two add support for updating
host_its_list from ACPI MADT table.
The rest patches provide support to update the hardware domain MADT table
with ITS information.

Changes since v3
- Set GICV3_ITS_SIZE as 128K
- updated gicv2_get_hwdom_madt_size
- Removed offset from gicv3_its_make_hwdom_madt

Changes since v2:
- %s/u32/unsigned long
- %s/u64/paddr_t
- cleanup gicv3_its_make_hwdom_madt as per review comments
- remove gicv3_its_host_has_its() checks in patch 3
- removed gicv3_its_madt_generic_translator_size() 

Changes since v1:
- split patches into smaller ones
- removed translation_id

Manish Jaggi (5):
  ARM: ITS: Introduce common function add_to_host_its_list
  ARM: ITS: Populate host_its_list from ACPI MADT Table
  ARM: ITS: Deny hardware domain access to ITS
  ARM: Introduce get_hwdom_madt_size in gic_hw_operations
  ARM: ITS: Expose ITS in the MADT table

 xen/arch/arm/domain_build.c  |  7 +---
 xen/arch/arm/gic-v2.c|  9 
 xen/arch/arm/gic-v3-its.c| 91 
 xen/arch/arm/gic-v3.c| 25 +++
 xen/arch/arm/gic.c   | 12 ++
 xen/include/asm-arm/gic.h|  3 ++
 xen/include/asm-arm/gic_v3_its.h | 27 
 7 files changed, 159 insertions(+), 15 deletions(-)

-- 
2.7.4


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

[Xen-devel] [PATCH v4 2/5] ARM: ITS: Populate host_its_list from ACPI MADT Table

[mjaggi]

From: Manish Jaggi 

Added gicv3_its_acpi_init to update host_its_list from MADT table.
For ACPI, host_its structure  stores dt_node as NULL.

Signed-off-by: Manish Jaggi 
---
 xen/arch/arm/gic-v3-its.c| 24 
 xen/arch/arm/gic-v3.c|  2 ++
 xen/include/asm-arm/gic_v3_its.h | 10 ++
 3 files changed, 36 insertions(+)

diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
index 0610991..0f662cf 100644
--- a/xen/arch/arm/gic-v3-its.c
+++ b/xen/arch/arm/gic-v3-its.c
@@ -18,6 +18,7 @@
  * along with this program; If not, see .
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -1018,6 +1019,29 @@ void gicv3_its_dt_init(const struct dt_device_node *node)
 }
 }
 
+#ifdef CONFIG_ACPI
+static int gicv3_its_acpi_probe(struct acpi_subtable_header *header,
+const unsigned long end)
+{
+struct acpi_madt_generic_translator *its;
+
+its = (struct acpi_madt_generic_translator *)header;
+if ( BAD_MADT_ENTRY(its, end) )
+return -EINVAL;
+
+add_to_host_its_list(its->base_address, GICV3_ITS_SIZE, NULL);
+
+return 0;
+}
+
+void gicv3_its_acpi_init(void)
+{
+/* Parse ITS information */
+acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_TRANSLATOR,
+gicv3_its_acpi_probe, 0);
+}
+#endif
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index f990eae..6f562f4 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1567,6 +1567,8 @@ static void __init gicv3_acpi_init(void)
 
 gicv3.rdist_stride = 0;
 
+gicv3_its_acpi_init();
+
 /*
  * In ACPI, 0 is considered as the invalid address. However the rest
  * of the initialization rely on the invalid address to be
diff --git a/xen/include/asm-arm/gic_v3_its.h b/xen/include/asm-arm/gic_v3_its.h
index 1fac1c7..e1be33c 100644
--- a/xen/include/asm-arm/gic_v3_its.h
+++ b/xen/include/asm-arm/gic_v3_its.h
@@ -20,6 +20,7 @@
 #ifndef __ASM_ARM_ITS_H__
 #define __ASM_ARM_ITS_H__
 
+#define GICV3_ITS_SIZE  SZ_128K
 #define GITS_CTLR   0x000
 #define GITS_IIDR   0x004
 #define GITS_TYPER  0x008
@@ -135,6 +136,9 @@ extern struct list_head host_its_list;
 /* Parse the host DT and pick up all host ITSes. */
 void gicv3_its_dt_init(const struct dt_device_node *node);
 
+#ifdef CONFIG_ACPI
+void gicv3_its_acpi_init(void);
+#endif
 bool gicv3_its_host_has_its(void);
 
 unsigned int vgic_v3_its_count(const struct domain *d);
@@ -196,6 +200,12 @@ static inline void gicv3_its_dt_init(const struct 
dt_device_node *node)
 {
 }
 
+#ifdef CONFIG_ACPI
+static inline void gicv3_its_acpi_init(void)
+{
+}
+#endif
+
 static inline bool gicv3_its_host_has_its(void)
 {
 return false;
-- 
2.7.4


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

[Xen-devel] [PATCH v4 5/5] ARM: ITS: Expose ITS in the MADT table

[mjaggi]

From: Manish Jaggi 

Add gicv3_its_make_hwdom_madt to update hwdom MADT ITS information.

Signed-off-by: Manish Jaggi 
---
 xen/arch/arm/gic-v3-its.c| 19 +++
 xen/arch/arm/gic-v3.c|  1 +
 xen/include/asm-arm/gic_v3_its.h |  8 
 3 files changed, 28 insertions(+)

diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
index 8697e5b..e3e7e92 100644
--- a/xen/arch/arm/gic-v3-its.c
+++ b/xen/arch/arm/gic-v3-its.c
@@ -1062,6 +1062,25 @@ void gicv3_its_acpi_init(void)
 acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_TRANSLATOR,
 gicv3_its_acpi_probe, 0);
 }
+
+unsigned long gicv3_its_make_hwdom_madt(const struct domain *d, void *base_ptr)
+{
+unsigned long i = 0;
+void *fw_its;
+struct acpi_madt_generic_translator *hwdom_its;
+
+hwdom_its = base_ptr;
+
+for ( i = 0; i < vgic_v3_its_count(d); i++ )
+{
+fw_its = acpi_table_get_entry_madt(ACPI_MADT_TYPE_GENERIC_TRANSLATOR,
+   i);
+memcpy(hwdom_its, fw_its, sizeof(struct acpi_madt_generic_translator));
+hwdom_its++;
+}
+
+return sizeof(struct acpi_madt_generic_translator) * vgic_v3_its_count(d);
+}
 #endif
 
 /*
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 6e8d580..d29eea6 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1403,6 +1403,7 @@ static int gicv3_make_hwdom_madt(const struct domain *d, 
u32 offset)
 table_len += size;
 }
 
+table_len += gicv3_its_make_hwdom_madt(d, base_ptr + table_len);
 return table_len;
 }
 
diff --git a/xen/include/asm-arm/gic_v3_its.h b/xen/include/asm-arm/gic_v3_its.h
index 31fca66..fc37776 100644
--- a/xen/include/asm-arm/gic_v3_its.h
+++ b/xen/include/asm-arm/gic_v3_its.h
@@ -138,6 +138,8 @@ void gicv3_its_dt_init(const struct dt_device_node *node);
 
 #ifdef CONFIG_ACPI
 void gicv3_its_acpi_init(void);
+unsigned long gicv3_its_make_hwdom_madt(const struct domain *d,
+void *base_ptr);
 #endif
 
 /* Deny iomem access for its */
@@ -208,6 +210,12 @@ static inline void gicv3_its_dt_init(const struct 
dt_device_node *node)
 static inline void gicv3_its_acpi_init(void)
 {
 }
+
+static inline unsigned long gicv3_its_make_hwdom_madt(const struct domain *d,
+  void *base_ptr)
+{
+return 0;
+}
 #endif
 
 static inline int gicv3_its_deny_access(const struct domain *d)
-- 
2.7.4


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

[Xen-devel] [PATCH v4 4/5] ARM: Introduce get_hwdom_madt_size in gic_hw_operations

[mjaggi]

From: Manish Jaggi 

estimate_acpi_efi_size needs to be updated to provide correct size of
hardware domains MADT, which now adds ITS information as well.

Introducing gic_get_hwdom_madt_size.

Signed-off-by: Manish Jaggi 
---
 xen/arch/arm/domain_build.c |  7 +--
 xen/arch/arm/gic-v2.c   |  9 +
 xen/arch/arm/gic-v3.c   | 19 +++
 xen/arch/arm/gic.c  | 12 
 xen/include/asm-arm/gic.h   |  3 +++
 5 files changed, 44 insertions(+), 6 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index d6f9585..f17fcf1 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1808,12 +1808,7 @@ static int estimate_acpi_efi_size(struct domain *d, 
struct kernel_info *kinfo)
 acpi_size = ROUNDUP(sizeof(struct acpi_table_fadt), 8);
 acpi_size += ROUNDUP(sizeof(struct acpi_table_stao), 8);
 
-madt_size = sizeof(struct acpi_table_madt)
-+ sizeof(struct acpi_madt_generic_interrupt) * d->max_vcpus
-+ sizeof(struct acpi_madt_generic_distributor);
-if ( d->arch.vgic.version == GIC_V3 )
-madt_size += sizeof(struct acpi_madt_generic_redistributor)
- * d->arch.vgic.nr_regions;
+madt_size = gic_get_hwdom_madt_size(d);
 acpi_size += ROUNDUP(madt_size, 8);
 
 addr = acpi_os_get_root_pointer();
diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index cbe71a9..2868766 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -1012,6 +1012,14 @@ static int gicv2_iomem_deny_access(const struct domain 
*d)
 return iomem_deny_access(d, mfn, mfn + nr);
 }
 
+static unsigned long gicv2_get_hwdom_madt_size(const struct domain *d)
+{
+return sizeof(struct acpi_table_madt)
++ sizeof(struct acpi_madt_generic_interrupt) * d->max_vcpus
++ sizeof(struct acpi_madt_generic_distributor);
+
+}
+
 #ifdef CONFIG_ACPI
 static int gicv2_make_hwdom_madt(const struct domain *d, u32 offset)
 {
@@ -1248,6 +1256,7 @@ const static struct gic_hw_operations gicv2_ops = {
 .read_apr= gicv2_read_apr,
 .make_hwdom_dt_node  = gicv2_make_hwdom_dt_node,
 .make_hwdom_madt = gicv2_make_hwdom_madt,
+.get_hwdom_madt_size = gicv2_get_hwdom_madt_size,
 .map_hwdom_extra_mappings = gicv2_map_hwdown_extra_mappings,
 .iomem_deny_access   = gicv2_iomem_deny_access,
 .do_LPI  = gicv2_do_LPI,
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index b3d605d..6e8d580 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1406,6 +1406,19 @@ static int gicv3_make_hwdom_madt(const struct domain *d, 
u32 offset)
 return table_len;
 }
 
+static unsigned long gicv3_get_hwdom_madt_size(const struct domain *d)
+{
+unsigned long size;
+
+size  = sizeof(struct acpi_madt_generic_redistributor)
+* d->arch.vgic.nr_regions;
+
+size  += vgic_v3_its_count(d)
+* sizeof(struct acpi_madt_generic_translator);
+
+return size;
+}
+
 static int __init
 gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header,
 const unsigned long end)
@@ -1597,6 +1610,11 @@ static int gicv3_make_hwdom_madt(const struct domain *d, 
u32 offset)
 {
 return 0;
 }
+
+static unsigned long gicv3_get_hwdom_madt_size(const struct domain *d)
+{
+return 0;
+}
 #endif
 
 /* Set up the GIC */
@@ -1698,6 +1716,7 @@ static const struct gic_hw_operations gicv3_ops = {
 .secondary_init  = gicv3_secondary_cpu_init,
 .make_hwdom_dt_node  = gicv3_make_hwdom_dt_node,
 .make_hwdom_madt = gicv3_make_hwdom_madt,
+.get_hwdom_madt_size = gicv3_get_hwdom_madt_size,
 .iomem_deny_access   = gicv3_iomem_deny_access,
 .do_LPI  = gicv3_do_LPI,
 };
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 6c803bf..f3c1f0b 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -851,6 +851,18 @@ int gic_make_hwdom_madt(const struct domain *d, u32 offset)
 return gic_hw_ops->make_hwdom_madt(d, offset);
 }
 
+unsigned long gic_get_hwdom_madt_size(const struct domain *d)
+{
+unsigned long madt_size;
+
+madt_size = sizeof(struct acpi_table_madt)
++ sizeof(struct acpi_madt_generic_interrupt) * d->max_vcpus
++ sizeof(struct acpi_madt_generic_distributor)
++ gic_hw_ops->get_hwdom_madt_size(d);
+
+return madt_size;
+}
+
 int gic_iomem_deny_access(const struct domain *d)
 {
 return gic_hw_ops->iomem_deny_access(d);
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 6203dc5..3acdd6d 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -365,6 +365,8 @@ struct gic_hw_operations {
 int (*make_hwdom_madt)(const struct domain *d, u32 offset);
 /* Map extra GIC MMIO, irqs and other hw stuffs to the hardware domain. */
 int (*map_hwdom_extra_mappings)(struct domain *d);
+/* Query the size

Re: [Xen-devel] [PATCH v6 02/11] vpci: introduce basic handlers to trap accesses to the PCI config space

[Wei Liu]

On Tue, Sep 19, 2017 at 04:29:27PM +0100, Roger Pau Monne wrote:
> This functionality is going to reside in vpci.c (and the corresponding
> vpci.h header), and should be arch-agnostic. The handlers introduced
> in this patch setup the basic functionality required in order to trap
> accesses to the PCI config space, and allow decoding the address and
> finding the corresponding handler that should handle the access
> (although no handlers are implemented).
> 
> Note that the traps to the PCI IO ports registers (0xcf8/0xcfc) are
> setup inside of a x86 HVM file, since that's not shared with other
> arches.
> 
> A new XEN_X86_EMU_VPCI x86 domain flag is added in order to signal Xen
> whether a domain should use the newly introduced vPCI handlers, this
> is only enabled for PVH Dom0 at the moment.
> 
> A very simple user-space test is also provided, so that the basic
> functionality of the vPCI traps can be asserted. This has been proven
> quite helpful during development, since the logic to handle partial
> accesses or accesses that expand across multiple registers is not
> trivial.
> 
> The handlers for the registers are added to a linked list that's keep
> sorted at all times. Both the read and write handlers support accesses
> that expand across multiple emulated registers and contain gaps not
> emulated.
> 
> Signed-off-by: Roger Pau Monné 

I am afraid I don't know much about PCI so I can't do meaningful review
of this patch.

The change to libxl looks good to me so for that bit:

Acked-by: Wei Liu 


> ---
> Cc: Ian Jackson 
> Cc: Wei Liu 
> Cc: Jan Beulich 
> Cc: Andrew Cooper 
> Cc: Paul Durrant 
> ---
> Changes since v5:
>  - Use a spinlock per pci device.
>  - Use the recently introduced pci_sbdf_t type.
>  - Fix test harness to use the right handler type and the newly
>introduced lock.
>  - Move the position of the vpci sections in the linker scripts.
>  - Constify domain and pci_dev in vpci_{read/write}.
>  - Fix typos in comments.
>  - Use _XEN_VPCI_H_ as header guard.
> 
> Changes since v4:
> * User-space test harness:
>  - Do not redirect the output of the test.
>  - Add main.c and emul.h as dependencies of the Makefile target.
>  - Use the same rule to modify the vpci and list headers.
>  - Remove underscores from local macro variables.
>  - Add _check suffix to the test harness multiread function.
>  - Change the value written by every different size in the multiwrite
>test.
>  - Use { } to initialize the r16 and r20 arrays (instead of { 0 }).
>  - Perform some of the read checks with the local variable directly.
>  - Expand some comments.
>  - Implement a dummy rwlock.
> * Hypervisor code:
>  - Guard the linker script changes with CONFIG_HAS_PCI.
>  - Rename vpci_access_check to vpci_access_allowed and make it return
>bool.
>  - Make hvm_pci_decode_addr return the register as return value.
>  - Use ~3 instead of 0xfffc to remove the register offset when
>checking accesses to IO ports.
>  - s/head/prev in vpci_add_register.
>  - Add parentheses around & in vpci_add_register.
>  - Fix register removal.
>  - Change the BUGs in vpci_{read/write}_hw helpers to
>ASSERT_UNREACHABLE.
>  - Make merge_result static and change the computation of the mask to
>avoid using a uint64_t.
>  - Modify vpci_read to only read from hardware the not-emulated gaps.
>  - Remove the vpci_val union and use a uint32_t instead.
>  - Change handler read type to return a uint32_t instead of modifying
>a variable passed by reference.
>  - Constify the data opaque parameter of read handlers.
>  - Change the size parameter of the vpci_{read/write} functions to
>unsigned int.
>  - Place the array of initialization handlers in init.rodata or
>.rodata depending on whether late-hwdom is enabled.
>  - Remove the pci_devs lock, assume the Dom0 is well behaved and won't
>remove the device while trying to access it.
>  - Change the recursive spinlock into a rw lock for performance
>reasons.
> 
> Changes since v3:
> * User-space test harness:
>  - Fix spaces in container_of macro.
>  - Implement a dummy locking functions.
>  - Remove 'current' macro make current a pointer to the statically
>allocated vpcu.
>  - Remove unneeded parentheses in the pci_conf_readX macros.
>  - Fix the name of the write test macro.
>  - Remove the dummy EXPORT_SYMBOL macro (this was needed by the RB
>code only).
>  - Import the max macro.
>  - Test all possible read/write size combinations with all possible
>emulated register sizes.
>  - Introduce a test for register removal.
> * Hypervisor code:
>  - Use a sorted list in order to store the config space handlers.
>  - Remove some unneeded 'else' branches.
>  - Make the IO port handlers always return X86EMUL_OKAY, and set the
>data to all 1's in case of read failure (write are simply ignored).
>  - In hvm_select_ioreq_server reuse local variables when calling
>XEN_DMOP_PCI_SBDF.
>  - Store the pointers to the initialization functions in the .r

Re: [Xen-devel] [PATCH v6 07/11] xen: introduce rangeset_consume_ranges

[Wei Liu]

On Tue, Sep 19, 2017 at 04:29:32PM +0100, Roger Pau Monne wrote:
> This function allows to iterate over a rangeset while removing the
> processed regions.
> 
> It will be used by the following patches in order to store memory
> regions in rangesets, and remove them while iterating.
> 
> Signed-off-by: Roger Pau Monné 
> ---
> Cc: George Dunlap 
> Cc: Ian Jackson 
> Cc: Jan Beulich 
> Cc: Konrad Rzeszutek Wilk 
> Cc: Stefano Stabellini 
> Cc: Tim Deegan 
> Cc: Wei Liu 
> ---
> Changes since v5:
>  - New in this version.
> ---
>  xen/common/rangeset.c  | 28 
>  xen/include/xen/rangeset.h |  4 
>  2 files changed, 32 insertions(+)
> 
> diff --git a/xen/common/rangeset.c b/xen/common/rangeset.c
> index 6c6293c15c..fd4a6b3384 100644
> --- a/xen/common/rangeset.c
> +++ b/xen/common/rangeset.c
> @@ -298,6 +298,34 @@ int rangeset_report_ranges(
>  return rc;
>  }

I think you need to document the behaviour of this new function due to
its destructive nature.

Something like:

Iterate through the range within a range set. Call cb on each range
provided. Bail on first error. Destroy the range processed when cb
has consumed the whole range.

Though without reading further I don't know why cb will only consume
part of the range but not all of it all the time.

>  
> +int rangeset_consume_ranges(
> +struct rangeset *r,
> +int (*cb)(unsigned long s, unsigned long e, void *, unsigned long *c),
> +void *ctxt)
> +{
> +int rc = 0;
> +
> +write_lock(&r->lock);
> +while ( !rangeset_is_empty(r) )
> +{
> +unsigned long consumed = 0;
> +struct range *x = first_range(r);
> +
> +rc = cb(x->s, x->e, ctxt, &consumed);
> +
> +ASSERT(consumed <= x->e - x->s + 1);
> +x->s += consumed;
> +if ( x->s > x->e )
> +destroy_range(r, x);
> +
> +if ( rc )
> +break;
> +}
> +write_unlock(&r->lock);
> +
> +return rc;
> +}
> +
>  int rangeset_add_singleton(
>  struct rangeset *r, unsigned long s)
>  {
> diff --git a/xen/include/xen/rangeset.h b/xen/include/xen/rangeset.h
> index aa6408248b..dfdb193800 100644
> --- a/xen/include/xen/rangeset.h
> +++ b/xen/include/xen/rangeset.h
> @@ -67,6 +67,10 @@ bool_t __must_check rangeset_overlaps_range(
>  int rangeset_report_ranges(
>  struct rangeset *r, unsigned long s, unsigned long e,
>  int (*cb)(unsigned long s, unsigned long e, void *), void *ctxt);
> +int rangeset_consume_ranges(
> +struct rangeset *r,
> +int (*cb)(unsigned long s, unsigned long e, void *, unsigned long *c),
> +void *ctxt);
>  
>  /* Add/remove/query a single number. */
>  int __must_check rangeset_add_singleton(
> -- 
> 2.11.0 (Apple Git-81)
> 

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

Re: [Xen-devel] [PATCH] xen: support 52 bit physical addresses in pv guests

[Boris Ostrovsky]

On 09/21/2017 04:01 AM, Juergen Gross wrote:

Physical addresses on processors supporting 5 level paging can be up to
52 bits wide. For a Xen pv guest running on such a machine those
physical addresses have to be supported in order to be able to use any
memory on the machine even if the guest itself does not support 5 level
paging.

So when reading/writing a MFN from/to a pte don't use the kernel's
PTE_PFN_MASK but a new XEN_PTE_MFN_MASK allowing full 40 bit wide MFNs.


full 52 bits?



Signed-off-by: Juergen Gross 
---
  arch/x86/include/asm/xen/page.h | 11 ++-
  arch/x86/xen/mmu_pv.c   |  4 ++--
  2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/xen/page.h b/arch/x86/include/asm/xen/page.h
index 07b6531813c4..bcb8b193c8d1 100644
--- a/arch/x86/include/asm/xen/page.h
+++ b/arch/x86/include/asm/xen/page.h
@@ -26,6 +26,15 @@ typedef struct xpaddr {
phys_addr_t paddr;
  } xpaddr_t;
  
+#ifdef CONFIG_X86_64

+#define XEN_PHYSICAL_MASK  ((1UL << 52) - 1)



SME is not supported for PV guests but for consistency (and in case sme 
bit somehow gets set)

#define XEN_PHYSICAL_MASK   __sme_clr(((1UL << 52) - 1))

But the real question that I have is whether this patch is sufficient. 
We are trying to preserve more bits in mfn but then this mfn is used, 
say, in pte_pfn_to_mfn() to build a pte. Can we be sure that the pte 
won't be stripped of higher bits in native code (again, as an example, 
native_make_pte()) because we are compiled with 5LEVEL?


-boris




+#else
+#define XEN_PHYSICAL_MASK  __PHYSICAL_MASK
+#endif
+
+#define XEN_PTE_MFN_MASK   ((pteval_t)(((signed long)PAGE_MASK) & \
+   XEN_PHYSICAL_MASK))
+
  #define XMADDR(x) ((xmaddr_t) { .maddr = (x) })
  #define XPADDR(x) ((xpaddr_t) { .paddr = (x) })
  
@@ -277,7 +286,7 @@ static inline unsigned long bfn_to_local_pfn(unsigned long mfn)
  
  static inline unsigned long pte_mfn(pte_t pte)

  {
-   return (pte.pte & PTE_PFN_MASK) >> PAGE_SHIFT;
+   return (pte.pte & XEN_PTE_MFN_MASK) >> PAGE_SHIFT;
  }
  
  static inline pte_t mfn_pte(unsigned long page_nr, pgprot_t pgprot)

diff --git a/arch/x86/xen/mmu_pv.c b/arch/x86/xen/mmu_pv.c
index 509f560bd0c6..958d36d776d9 100644
--- a/arch/x86/xen/mmu_pv.c
+++ b/arch/x86/xen/mmu_pv.c
@@ -315,7 +315,7 @@ void xen_ptep_modify_prot_commit(struct mm_struct *mm, 
unsigned long addr,
  static pteval_t pte_mfn_to_pfn(pteval_t val)
  {
if (val & _PAGE_PRESENT) {
-   unsigned long mfn = (val & PTE_PFN_MASK) >> PAGE_SHIFT;
+   unsigned long mfn = (val & XEN_PTE_MFN_MASK) >> PAGE_SHIFT;
unsigned long pfn = mfn_to_pfn(mfn);
  
  		pteval_t flags = val & PTE_FLAGS_MASK;

@@ -1740,7 +1740,7 @@ static unsigned long __init m2p(phys_addr_t maddr)
  {
phys_addr_t paddr;
  
-	maddr &= PTE_PFN_MASK;

+   maddr &= XEN_PTE_MFN_MASK;
paddr = mfn_to_pfn(maddr >> PAGE_SHIFT) << PAGE_SHIFT;
  
  	return paddr;




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

Re: [Xen-devel] [PATCH] x86/hvm: Remove unnecessary is_hvm_domain() test in construct_vmcs()

[Roger Pau Monné]

On Wed, Sep 20, 2017 at 03:50:27PM -0400, Boris Ostrovsky wrote:
> It's a leftover from PVHv1 days.
> 
> Signed-off-by: Boris Ostrovsky 

Reviewed-by: Roger Pau Monné 

Thanks, Roger.

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

Re: [Xen-devel] [PATCH] xen: support 52 bit physical addresses in pv guests

[Juergen Gross]

On 21/09/17 16:14, Boris Ostrovsky wrote:
> 
> 
> On 09/21/2017 04:01 AM, Juergen Gross wrote:
>> Physical addresses on processors supporting 5 level paging can be up to
>> 52 bits wide. For a Xen pv guest running on such a machine those
>> physical addresses have to be supported in order to be able to use any
>> memory on the machine even if the guest itself does not support 5 level
>> paging.
>>
>> So when reading/writing a MFN from/to a pte don't use the kernel's
>> PTE_PFN_MASK but a new XEN_PTE_MFN_MASK allowing full 40 bit wide MFNs.
> 
> full 52 bits?

The MFN mask is only 40 bits. This plus the 12 bits page offset are 52
bits of machine address width.

> 
>>
>> Signed-off-by: Juergen Gross 
>> ---
>>   arch/x86/include/asm/xen/page.h | 11 ++-
>>   arch/x86/xen/mmu_pv.c   |  4 ++--
>>   2 files changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/xen/page.h
>> b/arch/x86/include/asm/xen/page.h
>> index 07b6531813c4..bcb8b193c8d1 100644
>> --- a/arch/x86/include/asm/xen/page.h
>> +++ b/arch/x86/include/asm/xen/page.h
>> @@ -26,6 +26,15 @@ typedef struct xpaddr {
>>   phys_addr_t paddr;
>>   } xpaddr_t;
>>   +#ifdef CONFIG_X86_64
>> +#define XEN_PHYSICAL_MASK    ((1UL << 52) - 1)
> 
> 
> SME is not supported for PV guests but for consistency (and in case sme
> bit somehow gets set)
> #define XEN_PHYSICAL_MASK    __sme_clr(((1UL << 52) - 1))

Hmm, really? Shouldn't we rather add something like

BUG_ON(sme_active());

somewhere?

> But the real question that I have is whether this patch is sufficient.
> We are trying to preserve more bits in mfn but then this mfn is used,
> say, in pte_pfn_to_mfn() to build a pte. Can we be sure that the pte
> won't be stripped of higher bits in native code (again, as an example,
> native_make_pte()) because we are compiled with 5LEVEL?

native_make_pte() just encapsulates pte_t. It doesn't modify the value
of the pte at all.

Physical address bits are only ever masked away via PTE_PFN_MASK and I
haven't found any place where it is used for a MFN other than those I
touched in this patch.


Juergen

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

Re: [Xen-devel] [PATCH v6 07/11] xen: introduce rangeset_consume_ranges

[Roger Pau Monné]

On Thu, Sep 21, 2017 at 02:53:54PM +0100, Wei Liu wrote:
> On Tue, Sep 19, 2017 at 04:29:32PM +0100, Roger Pau Monne wrote:
> > This function allows to iterate over a rangeset while removing the
> > processed regions.
> > 
> > It will be used by the following patches in order to store memory
> > regions in rangesets, and remove them while iterating.
> > 
> > Signed-off-by: Roger Pau Monné 
> > ---
> > Cc: George Dunlap 
> > Cc: Ian Jackson 
> > Cc: Jan Beulich 
> > Cc: Konrad Rzeszutek Wilk 
> > Cc: Stefano Stabellini 
> > Cc: Tim Deegan 
> > Cc: Wei Liu 
> > ---
> > Changes since v5:
> >  - New in this version.
> > ---
> >  xen/common/rangeset.c  | 28 
> >  xen/include/xen/rangeset.h |  4 
> >  2 files changed, 32 insertions(+)
> > 
> > diff --git a/xen/common/rangeset.c b/xen/common/rangeset.c
> > index 6c6293c15c..fd4a6b3384 100644
> > --- a/xen/common/rangeset.c
> > +++ b/xen/common/rangeset.c
> > @@ -298,6 +298,34 @@ int rangeset_report_ranges(
> >  return rc;
> >  }
> 
> I think you need to document the behaviour of this new function due to
> its destructive nature.
> 
> Something like:
> 
> Iterate through the range within a range set. Call cb on each range
> provided. Bail on first error. Destroy the range processed when cb
> has consumed the whole range.

OK, I thought that the 'consume' in the name was enough, but now that
you have written the comment I certainly don't mind adding it ;).

> Though without reading further I don't know why cb will only consume
> part of the range but not all of it all the time.

I guess you have to look at the next patch and it's usage. This will
be used to store all the MMIO areas that need to be mapped into a
domain p2m.

Some of the ranges might be very big (BARs from gfx cards for
example), and might require preemption in order to map them, hence the
emulated PCI code needs a way to store it's progress, and that's done
by partially consuming a range.

Thanks, Roger.

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

Re: [Xen-devel] [PATCH v2 2/3] Introduce migration precopy policy

[Roger Pau Monné]

On Thu, Sep 21, 2017 at 12:13:55PM +0100, Wei Liu wrote:
> On Thu, Sep 21, 2017 at 12:08:04PM +0100, Roger Pau Monné wrote:
> > On Wed, Sep 20, 2017 at 05:18:16PM +0100, Jennifer Herbert wrote:
> > > On 20/09/17 11:20, Roger Pau Monné wrote:
> > > > On Tue, Sep 19, 2017 at 07:06:26PM +0100, Jennifer Herbert wrote:
> > > > > +? XGS_POLICY_STOP_AND_COPY
> > > > > +: XGS_POLICY_CONTINUE_PRECOPY;
> > > > > +}
> > > > > +
> > > > > +/*
> > > > >* Send memory while guest is running.
> > > > >*/
> > > > >   static int send_memory_live(struct xc_sr_context *ctx)
> > > > > @@ -474,21 +491,58 @@ static int send_memory_live(struct 
> > > > > xc_sr_context *ctx)
> > > > >   xc_interface *xch = ctx->xch;
> > > > >   xc_shadow_op_stats_t stats = { 0, ctx->save.p2m_size };
> > > > >   char *progress_str = NULL;
> > > > > -unsigned x;
> > > > > +unsigned int x = 0;
> > > > >   int rc;
> > > > > +int policy_decision;
> > > > > +
> > > > > +DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap,
> > > > > +&ctx->save.dirty_bitmap_hbuf);
> > > > > +
> > > > > +precopy_policy_t precopy_policy = 
> > > > > ctx->save.callbacks->precopy_policy;
> > > > > +void *data = ctx->save.callbacks->data;
> > > > > +
> > > > > +struct precopy_stats *policy_stats;
> > > > >   rc = update_progress_string(ctx, &progress_str, 0);
> > > > >   if ( rc )
> > > > >   goto out;
> > > > > -rc = send_all_pages(ctx);
> > > > > -if ( rc )
> > > > > -goto out;
> > > > > +ctx->save.stats = (struct precopy_stats)
> > > > > +{ .dirty_count   = ctx->save.p2m_size };
> > > > This is exactly the same as 'stats' at this point. I'm slightly
> > > > confused about why you need 2 different stats variable, plus a pointer
> > > > to a stats variable (stats, ctx->save.stats and *policy_stats).
> > > 
> > > They do start off similar, and are certainly closely related.
> > > xc_shadow_op_stats_t stats has different fields in it then precopy_stats
> > > policy_stats.
> > > The former has a fault and dirty count, per iteration, while the latter 
> > > has
> > > iteration number, total_written (over all iterations) and dirty count.
> > 
> > OK. I'm not that familiar with this code, so maybe this doesn't make
> > sense, but wouldn't it be clearer to expand the xc_shadow_op_stats_t
> > type so that a single variable can contain all this information?
> > 
> > I find it slightly confusing to use two variables of the same type
> > that track different things.
> > 
> 
> The xc_shadow_op_stats_t is in fact xen_domctl_shadow_op_stats, which
> gets passed directly to the hypervisor. So I think having two separate
> structs here is okay. They are describing different things after all.

You could have one structure nested inside of the other, but I don't
have such a strong opinion, ie: this is fine.

Thanks, Roger.

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

Re: [Xen-devel] [PATCH v5 06/10] arm: smccc: handle SMCs according to SMCCC

[Julien Grall]

On 20/09/17 21:26, Volodymyr Babchuk wrote:



On 20.09.17 23:02, Julien Grall wrote:



On 20/09/2017 19:11, Volodymyr Babchuk wrote:

On 20.09.17 20:21, Julien Grall wrote:



On 19/09/17 22:44, Volodymyr Babchuk wrote:

Hi Julien,


Hi Volodymyr,



On 13.09.17 14:11, Julien Grall wrote:

Hi,

On 08/31/2017 09:09 PM, Volodymyr Babchuk wrote:



+static void fill_uuid(struct cpu_user_regs *regs, const xen_uuid_t

*u)


Actually why do you pass a pointer for u? This requires every caller
to introduce temporary variable because the UUID is usually a define.

Hmm, another way probably is to pass a whole structure as a parameter.
Are you suggesting this approach? Something like
fill_uuid(regs, (xen_uuid_t)MY_UUID)?


Something list that. But why do you need the cast? MY_UUID is supposed
to be a xen_uuid_t. No?

It have no type. It is just an initializer list like {1,2,3,4,5,6}. If
you remember that thread, there is a requirement to make public headers
compatible with c89. So I can't define MY_UUID as (xen_uuid_t){1,2,3}.
Instead it is defined as a plain initializer list.


In that case why don't introduce a version for non-strict ansi? This 
would introduce a bit of safety and avoid cast a bit unexplained. (see 
how __DECL_REG(...,...) is done in include/public/asm-arm.h?

I believe you meant arch-arm.h.

Just to be clear, you are proposing to introduce one
#define XEN_DEFINE_UUID in a public header, and another one in a private 
header?


No the two in the public header. One version for strict-ansi compiler, 
the other for gcc-compatible one.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v2 02/16] xen/x86: p2m-pod: Remove trailing whitespaces

[Wei Liu]

On Thu, Sep 21, 2017 at 01:40:21PM +0100, Julien Grall wrote:
> Signed-off-by: Julien Grall 
> Acked-by: Andrew Cooper 
> 

Reviewed-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v2 01/16] xen/x86: p2m-pod: Clean-up includes

[Wei Liu]

On Thu, Sep 21, 2017 at 01:40:20PM +0100, Julien Grall wrote:
> A lot of the headers are not necessary. At the same time, order them in the
> alphabetical order.
> 
> Signed-off-by: Julien Grall 
> Acked-by: Andrew Cooper 

Reviewed-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v2 03/16] xen/x86: p2m-pod: Fix coding style for comments

[Wei Liu]

On Thu, Sep 21, 2017 at 01:40:22PM +0100, Julien Grall wrote:
> Signed-off-by: Julien Grall 
> Acked-by: Andrew Cooper 

Reviewed-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v2 04/16] xen/x86: p2m-pod: Fix coding style

[Wei Liu]

On Thu, Sep 21, 2017 at 01:40:23PM +0100, Julien Grall wrote:
> Also take the opportunity to:
> - move from 1 << * to 1UL << *.
> - use unsigned when possible
> - move from unsigned int -> unsigned long for some induction
> variables
> 
> Signed-off-by: Julien Grall 
> Acked-by: Andrew Cooper 

Reviewed-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v2 05/16] xen/x86: p2m-pod: Avoid redundant assignments in p2m_pod_demand_populate

[Wei Liu]

On Thu, Sep 21, 2017 at 01:40:24PM +0100, Julien Grall wrote:
> gfn_aligned is assigned 3 times with the exact same formula. All the
> variables used are not modified, so consolidate in a single assignment
> at the beginning of the function.
> 
> Signed-off-by: Julien Grall 
> Acked-by: Andrew Cooper 

Reviewed-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v2 06/16] xen/x86: p2m-pod: Clean-up use of typesafe MFN

[Wei Liu]

On Thu, Sep 21, 2017 at 01:40:25PM +0100, Julien Grall wrote:
> Some unboxing/boxing can be avoided by using mfn_add(...) instead.
> 
> Signed-off-by: Julien Grall 
> Acked-by: Andrew Cooper 

Reviewed-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v4 10/11] livepatch/arm[32, 64]: Modify .livepatch.funcs section to be RW when patching

[Julien Grall]

Hi Jan,

On 21/09/17 13:16, Jan Beulich wrote:

On 21.09.17 at 00:31,  wrote:

@@ -43,7 +46,29 @@ int arch_livepatch_quiesce(void)
  return -ENOMEM;
  }
  
-return 0;

+if ( nfuncs )
+{
+unsigned long va = (unsigned long)func;
+unsigned int offs = va & (PAGE_SIZE - 1);
+unsigned int pages = PFN_UP(offs + nfuncs * sizeof(*func));
+
+va &= PAGE_MASK;
+
+rc = modify_xen_mappings(va, va + (pages * PAGE_SIZE), PTE_NX);
+if ( rc )
+{
+printk(XENLOG_ERR LIVEPATCH "Failed to modify 0x%lx to RW\n", va);


%#lx ?


+vunmap(vmap_of_xen_text);
+vmap_of_xen_text = NULL;
+}
+else
+{
+livepatch_stash.va = va;
+livepatch_stash.pages = pages;
+}
+}


You're effectively doing all this behind the back of vmalloc() / vmap();
I'm not sure this is a good idea, but I'm also not a maintainer of this
code.


We already have place in the code (both x86 and Arm) modifying memory 
attributes on the back of vmalloc/vmap. See arch_livepatch_secure for 
instance.


I suggested this solution because it avoids to create a temporary 
mapping for the .livepatch.funcs section.


Do you foresee potentially issue of temporarily modifying permissions of 
a mapping?


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH 1/2] common/efi: bail if dom0 fails the shim verification step

[Tamas K Lengyel]

On Thu, Sep 21, 2017 at 7:03 AM, Jan Beulich  wrote:
 On 20.09.17 at 22:57,  wrote:
>> --- a/xen/common/efi/boot.c
>> +++ b/xen/common/efi/boot.c
>> @@ -1226,9 +1226,13 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE 
>> *SystemTable)
>>  efi_bs->FreePool(name.w);
>>
>>  if ( !EFI_ERROR(efi_bs->LocateProtocol(&shim_lock_guid, NULL,
>> -(void **)&shim_lock)) &&
>> - (status = shim_lock->Verify(kernel.ptr, kernel.size)) != 
>> EFI_SUCCESS )
>> -PrintErrMesg(L"Dom0 kernel image could not be verified", 
>> status);
>> +(void **)&shim_lock)))
>> +{
>> +if  ( shim_lock->Verify(kernel.ptr, kernel.size) != EFI_SUCCESS 
>> )
>> +blexit(L"Dom0 kernel image could not be verified by the 
>> shim.");
>> +
>> +PrintStr(L"Dom0 kernel image was verified by the shim.\r\n");
>> +}
>
> So what is the actual behavioral change you're trying to
> accomplish? PrintErrMesg() already calls blexit(),

Indeed, I've somehow missed that. Sorry for the noise.

Tamas

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

Re: [Xen-devel] [PATCH v4 10/11] livepatch/arm[32, 64]: Modify .livepatch.funcs section to be RW when patching

[Jan Beulich]

>>> On 21.09.17 at 16:58,  wrote:
> Do you foresee potentially issue of temporarily modifying permissions of 
> a mapping?

It's generally not a good idea imo, but perhaps it's fine here. I
assume the page permissions can't be adversely affected despite
their hard coding in the function invocations.

Jan


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

Re: [Xen-devel] [PATCH v2 16/24] xen/arm: page: Use ARMv8 naming to improve readability

[Julien Grall]

Hi,

On 20/09/17 00:45, Stefano Stabellini wrote:

diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
index 30fcfa0778..899fd1801a 100644
--- a/xen/include/asm-arm/page.h
+++ b/xen/include/asm-arm/page.h
@@ -26,14 +26,14 @@
   * LPAE entry; the 8-bit fields are packed little-endian into MAIR0 and MAIR1.
   *
   *aiencoding
- *   MT_UNCACHED  000      -- Strongly Ordered
- *   MT_BUFFERABLE001   0100 0100  -- Non-Cacheable
- *   MT_WRITETHROUGH  010   1010 1010  -- Write-through
- *   MT_WRITEBACK 011   1110 1110  -- Write-back
- *   MT_DEV_SHARED100    0100  -- Device
+ *   MT_DEVICE_nGnRE  000      -- Strongly Ordered/Device nGnRnE


I admit I always hated the "nGnRE" acronym. However, it is on the ARM
ARM too, so if you'd like to introduce it here, I'll accept it. But
please at least expand the acronym in the comment to make it
understandable (same with nGnRnE).


"nGnRE" acronym are not great but convey the meaning of what would be 
the resulting attribute. For instance MT_UNCACHED does not really say if 
it is for device or memory. Lets not even mention MT_BUFFERABLE which is 
in fact non-cacheable memory :).




Also, the comment say "nGnRnE" while the definition is MT_DEVICE_nGnRE.


Actually, the comment is correct but not the naming. It should 
MT_DEVICE_nGnRnE. I will rename it.


Aside that, I think the comment is understandable. nGnRnE is equivalent 
to Strongly ordered. I could expand nGnRnE (non-Gatherable, 
non-Reordering, No Early write acknowledgment) but I feel at this stage 
you can just search the name in the ARM ARM...






+ *   MT_NORMAL_NC 001   0100 0100  -- Non-Cacheable
+ *   MT_NORMAL_WT 010   1010 1010  -- Write-through
+ *   MT_NORMAL_WB 011   1110 1110  -- Write-back
+ *   MT_DEVICE_nGnRE  100    0100  -- Device nGnRE
   *   ??   101
   *   reserved 110
- *   MT_WRITEALLOC111      -- Write-back write-allocate
+ *   MT_NORMAL111      -- Write-back write-allocate
   */
  #define MAIR0VAL 0xeeaa4400
  #define MAIR1VAL 0xff04
@@ -47,16 +47,16 @@
   * registers, as defined above.
   *
   */
-#define MT_UNCACHED  0x0
-#define MT_BUFFERABLE0x1
-#define MT_WRITETHROUGH  0x2
-#define MT_WRITEBACK 0x3
-#define MT_DEV_SHARED0x4
-#define MT_WRITEALLOC0x7
-
-#define PAGE_HYPERVISOR (MT_WRITEALLOC)
-#define PAGE_HYPERVISOR_NOCACHE (MT_DEV_SHARED)
-#define PAGE_HYPERVISOR_WC  (MT_BUFFERABLE)
+#define MT_DEVICE_nGnRnE 0x0
+#define MT_NORMAL_NC 0x1
+#define MT_NORMAL_WT 0x2
+#define MT_NORMAL_WB 0x3
+#define MT_DEVICE_nGnRE  0x4
+#define MT_NORMAL0x7
+
+#define PAGE_HYPERVISOR (MT_NORMAL)
+#define PAGE_HYPERVISOR_NOCACHE (MT_DEVICE_nGnRE)
+#define PAGE_HYPERVISOR_WC  (MT_NORMAL_NC)
  
  /*

   * Defines for changing the hypervisor PTE .ro and .nx bits. This is only to 
be


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v2 07/16] xen/x86: p2m-pod: Use typesafe gfn in p2m_pod_decrease_reservation

[Wei Liu]

On Thu, Sep 21, 2017 at 01:40:26PM +0100, Julien Grall wrote:
> Signed-off-by: Julien Grall 
> Acked-by: Andrew Cooper 

I wonder how gfn_lock work with the new time without any change, but it
appears gfn_lock ignores gfn completely. :-)

Reviewed-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v2 08/16] xen/x86: p2m: Use typesafe gfn for the P2M callbacks get_entry and set_entry

[Wei Liu]

On Thu, Sep 21, 2017 at 01:40:27PM +0100, Julien Grall wrote:
> Signed-off-by: Julien Grall 
> Acked-by: Andrew Cooper 
> Reviewed-by: Kevin Tian 
> 

Reviewed-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v2 09/16] xen/x86: p2m: Use typesafe GFN in p2m_set_entry

[Wei Liu]

On Thu, Sep 21, 2017 at 01:40:28PM +0100, Julien Grall wrote:
> Signed-off-by: Julien Grall 
> Acked-by: Andrew Cooper 
> Acked-by: Tamas K Lengyel 
> 

Reviewed-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v2 10/16] xen/x86: p2m-pod: Use typesafe GFN in pod_eager_record

[Wei Liu]

On Thu, Sep 21, 2017 at 01:40:29PM +0100, Julien Grall wrote:
> Signed-off-by: Julien Grall 
> Acked-by: Andrew Cooper 

Reviewed-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v2 11/16] xen/x86: p2m-pod: Clean-up p2m_pod_zero_check

[Wei Liu]

On Thu, Sep 21, 2017 at 01:40:30PM +0100, Julien Grall wrote:
> Signed-off-by: Julien Grall 
> Acked-by: Andrew Cooper 

Reviewed-by: Wei Liu 

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