[Xen-devel] Running Driver Domain in different mode and hypervisor

[tusherpat]

Hi Guys,  lately, I have been trying to play with Xen Driver Domain. I
have been able to make it work where both the  Driver Domain OS and
the guest OS are run on paravirtualized (PV) machine. However, it
doesn't work when any of them are hardware virtualized machine (HVM).
Therefore, I have the following questions now.  1. Is it possible to
have the Driver Domain and guests, who are using the corresponding
drivers, run on virtual machines using other than the PV mode, like
HVM or PVHVM? If yes, then what are the possible combinations?  2. Is
it possible to use virtIO for paravirtualization where the underlying
hypervisor is Xen? If yes, then can we run Driver Domain using virtIO?
 3. Is it possible to have a Driver Domain where the underlying
hypervisor is KVM instead of Xen?  The last question may not be
directly related to Xen. However, I am tempted to ask as good things
like PV was first introduced in Xen then others hypervisors adapted
this design.   Please let me know your valuable opinion and feel free
to provide any link where I can study further regarding these matters.
Thanks, Mehrab.

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

[Xen-devel] Running Driver Domain in different mode and hypervisor

[tosher 1]

Hi Guys,
Lately, I have been trying to play with Xen Driver Domain. I have been able to 
make it work where both the  Driver Domain OS and the guest OS are run on 
paravirtualized (PV) machine. However, it doesn't work when any of them are 
hardware virtualized machine (HVM). Therefore, I have the following questions 
now.
1. Is it possible to have the Driver Domain and guests, who are using the 
corresponding drivers, run on virtual machines using other than the PV mode, 
like HVM or PVHVM? If yes, then what are the possible combinations?
2. Is it possible to use virtIO for paravirtualization where the underlying 
hypervisor is Xen? If yes, then can we run Driver Domain using virtIO?
3. Is it possible to have a Driver Domain where the underlying hypervisor is 
KVM instead of Xen?

The last question may not be directly related to Xen. However, I am tempted to 
ask as good things like PV was first introduced in Xen then others hypervisors 
adapted this design.  

Please let me know your valuable opinion and feel free to provide any link 
where I can study further regarding these matters.
Thanks,Mehrab___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 6/6] x86/vtd: Drop struct intel_iommu

[Tian, Kevin]

> From: Andrew Cooper [mailto:andrew.coop...@citrix.com]
> Sent: Saturday, February 23, 2019 3:13 AM
> 
> The sole remaining member of struct intel_iommu is the drhd backpointer.
> Move
> this into struct vtd_iommu, replacing the the 'intel' pointer.
> 
> This removes one dynamic memory allocation per IOMMU on the system.
> 
> Signed-off-by: Andrew Cooper 

Acked-by: Kevin Tian 

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

Re: [Xen-devel] [PATCH 5/6] x86/vtd: Drop struct iommu_flush

[Tian, Kevin]

> From: Andrew Cooper [mailto:andrew.coop...@citrix.com]
> Sent: Saturday, February 23, 2019 3:13 AM
> 
> It is unclear why this abstraction exists, but iommu_get_flush() returns
> possibly NULL and every user unconditionally dereferences the result.  In
> practice, I can't spot a path where iommu is NULL, so I think it is mostly
> dead.
> 
> Move the two function pointers into struct vtd_iommu (using a flush_ prefix),
> and delete iommu_get_flush().  Furthermore, there is no need to pass the
> IOMMU
> pointer to the callbacks via a void pointer, so change the parameter to be
> correctly typed as struct vtd_iommu.  Clean up bool_t to bool in surrounding
> context.
> 
> Signed-off-by: Andrew Cooper 

Acked-by: Kevin Tian 

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

Re: [Xen-devel] [PATCH 4/6] x86/vtd: Drop struct ir_ctrl

[Tian, Kevin]

> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: Monday, February 25, 2019 7:01 PM
> 
> >>> On 25.02.19 at 11:09,  wrote:
> >>  -Original Message-
> > [snip]
> >>  struct iommu_flush {
> >>  int __must_check (*context)(void *iommu, u16 did, u16 source_id,
> >>  u8 function_mask, u64 type,
> >> @@ -523,7 +517,6 @@ struct iommu_flush {
> >>  };
> >>
> >>  struct intel_iommu {
> >> -struct ir_ctrl ir_ctrl;
> >>  struct iommu_flush flush;
> >>  struct acpi_drhd_unit *drhd;
> >>  };
> >> @@ -543,16 +536,15 @@ struct vtd_iommu {
> >>
> >>  uint64_t qinval_maddr;   /* queue invalidation page machine address
> */
> >>
> >> +uint64_t iremap_maddr;   /* interrupt remap table machine address */
> >> +unsigned int iremap_num; /* total num of used interrupt remap entry
> */
> >> +spinlock_t iremap_lock;  /* lock for irq remapping table */
> >> +
> >
> > Elsewhere in the Xen codebase we try to group related fields, so how ahout
> > the following?
> >
> > struct {
> > uint64_t maddr;   /* interrupt remap table machine address */
> > unsigned int num; /* total num of used interrupt remap entry */
> > spinlock_t lock;  /* lock for irq remapping table */
> > } iremap;
> >
> > You'd than have a fairly mechanical job of replacing '_' with '.' in a few
> > places in your patch but I think it would look better overall.
> 
> +1
> 

agree.

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

Re: [Xen-devel] [PATCH 3/6] x86/vtd: Drop struct qi_ctrl

[Tian, Kevin]

> From: Andrew Cooper [mailto:andrew.coop...@citrix.com]
> Sent: Saturday, February 23, 2019 3:13 AM
> 
> It is unclear why this abstraction exists, but iommu_qi_ctrl() returns
> possibly NULL and every user unconditionally dereferences the result.  In
> practice, I can't spot a path where iommu is NULL, so I think it is mostly
> dead.
> 
> Move the sole member into struct vtd_iommu, and delete iommu_qi_ctrl().
> 
> Signed-off-by: Andrew Cooper 

Acked-by: Kevin Tian 

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

Re: [Xen-devel] [PATCH 4/4] x86/vmx: Properly flush the TLB when an altp2m is modified

[Tian, Kevin]

> From: Andrew Cooper [mailto:andrew.coop...@citrix.com]
> Sent: Wednesday, February 20, 2019 6:19 AM
> 
> Modificaitons to an altp2m mark the p2m as needing flushing, but this was

Modifications

> never wired up in the return-to-guest path.  As a result, stale TLB entries
> can remain after resuming the guest.
> 
> In practice, this manifests as a missing EPT_VIOLATION or #VE exception
> when
> the guest subsequently accesses a page which has had its permissions
> reduced.
> 
> vmx_vmenter_helper() now has 11 p2ms to potentially invalidate, but issuing
> 11
> INVEPT instructions isn't clever.  Instead, count how many contexts need
> invalidating, and use INVEPT_ALL_CONTEXT if two or more are in need of
> flushing.
> 
> This doesn't have an XSA because altp2m is not yet a security-supported
> feature.
> 
> Signed-off-by: Andrew Cooper 

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

Re: [Xen-devel] [PATCH v2 3/4] x86/vmx: Fix security issue when a guest balloons out the #VE info page

[Tian, Kevin]

> From: Andrew Cooper [mailto:andrew.coop...@citrix.com]
> Sent: Friday, February 22, 2019 4:19 AM
> 
> The logic in altp2m_vcpu_{en,dis}able_ve() and
> vmx_vcpu_update_vmfunc_ve() is
> dangerous.  After #VE has been set up, the guest can balloon out and free the
> nominated GFN, after which the processor may write to it.  Also, the unlocked
> GFN query means the MFN is stale by the time it is used.  Alternatively, a
> guest can race two disable calls to cause one VMCS to still reference the
> nominated GFN after the tracking information was dropped.
> 
> Rework the logic from scratch to make it safe.
> 
> Hold an extra page reference on the underlying frame, to account for the
> VMCS's reference.  This means that if the GFN gets ballooned out, it isn't
> freed back to Xen until #VE is disabled, and the VMCS no longer refers to the
> page.
> 
> A consequence of this is that altp2m_vcpu_disable_ve() needs to be called
> during the domain_kill() path, to drop the reference for domains which shut
> down with #VE still enabled.
> 
> For domains using altp2m, we expect a single enable call and no disable for
> the remaining lifetime of the domain.  However, to avoid problems with
> concurrent calls, use cmpxchg() to locklessly maintain safety.
> 
> This doesn't have an XSA because altp2m is not yet a security-supported
> feature.
> 
> Signed-off-by: Andrew Cooper 
> Release-acked-by: Juergen Gross 

Reviewed-by: Kevin Tian 

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

[Xen-devel] [xen-4.9-testing test] 133435: trouble: blocked/broken/pass

[osstest service owner]

flight 133435 xen-4.9-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/133435/

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-i386-prev  broken
 build-amd64-xsm  broken
 build-arm64-xsm  broken
 build-amd64  broken
 build-armhf-pvopsbroken
 build-arm64  broken
 build-i386-pvops broken
 build-armhf  broken
 build-amd64-pvopsbroken
 build-amd64-xtf  broken
 build-amd64-prev broken
 build-i386   broken
 build-arm64   4 host-install(4)broken REGR. vs. 132889
 build-amd64-xsm   4 host-install(4)broken REGR. vs. 132889
 build-amd64-pvops 4 host-install(4)broken REGR. vs. 132889
 build-amd64   4 host-install(4)broken REGR. vs. 132889
 build-amd64-prev  4 host-install(4)broken REGR. vs. 132889
 build-arm64-xsm   4 host-install(4)broken REGR. vs. 132889
 build-amd64-xtf   4 host-install(4)broken REGR. vs. 132889
 build-i3864 host-install(4)broken REGR. vs. 132889
 build-i386-prev   4 host-install(4)broken REGR. vs. 132889
 build-i386-pvops  4 host-install(4)broken REGR. vs. 132889
 build-armhf   4 host-install(4)broken REGR. vs. 132889
 build-armhf-pvops 4 host-install(4)broken REGR. vs. 132889

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow  1 build-check(1) blocked n/a
 test-amd64-amd64-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl1 build-check(1)   blocked  n/a
 test-amd64-i386-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a
 test-amd64-amd64-xl-qemuu-ws16-amd64  1 build-check(1) blocked n/a
 test-arm64-arm64-xl-credit1   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm  1 build-check(1)blocked n/a
 test-amd64-i386-qemut-rhel6hvm-intel  1 build-check(1) blocked n/a
 test-amd64-amd64-livepatch1 build-check(1)   blocked  n/a
 test-amd64-amd64-rumprun-amd64  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl   1 build-check(1)   blocked  n/a
 test-xtf-amd64-amd64-21 build-check(1)   blocked  n/a
 test-amd64-i386-qemuu-rhel6hvm-amd  1 build-check(1)   blocked n/a
 test-amd64-i386-xl-shadow 1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-xsm1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemut-win7-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow  1 build-check(1)  blocked n/a
 test-amd64-amd64-xl-rtds  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-cubietruck  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 build-armhf-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-xsm   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm  1 build-check(1)blocked n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  1 build-check(1)blocked n/a
 test-amd64-i386-xl-qemut-win10-i386  1 build-check(1)  blocked n/a
 test-xtf-amd64-amd64-41 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-arndale   1 build-check(1)   blocked  n/a
 test-amd64-amd64-pair 1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-credit1   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-debianhvm-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-libvirt-xsm   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-credit2   1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 

[Xen-devel] [ovmf test] 133459: all pass - PUSHED

[osstest service owner]

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

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 8b8d6f8a3beea391b1ec39ac347ef69501b44019
baseline version:
 ovmf c417c1b33d06ef6ae96adb373201a5a3c3b38772

Last test of basis   133291  2019-02-18 01:41:15 Z   10 days
Failing since133305  2019-02-19 00:41:21 Z9 days   10 attempts
Testing same since   133459  2019-02-27 18:41:23 Z0 days1 attempts


People who touched revisions under test:
  Albecki Mateusz 
  Albecki, Mateusz 
  Anthony PERARD 
  Ard Biesheuvel 
  Ashish Singhal 
  Bob Feng 
  Chasel Chiu 
  Chasel, Chiu 
  Chen A Chen 
  Dandan Bi 
  Edgar Handal 
  Fan, ZhijuX 
  Feng, Bob C 
  Gonzalez Del Cueto, Rodrigo 
  Hao Wu 
  Jeff Brasen 
  Jian J Wang 
  Jiaxin Wu 
  Jiewen Yao 
  Jordan Justen 
  Julien Grall 
  Laszlo Ersek 
  Liming Gao 
  Max Knutsen 
  Pete Batard 
  Ray Ni 
  Rodrigo Gonzalez del Cueto 
  Ruiyu Ni 
  Sami Mujawar 
  Shenglei Zhang 
  Star Zeng 
  Wu Jiaxin 
  Zhichao Gao 
  Zhiju.Fan 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 pass
 test-amd64-i386-xl-qemuu-ovmf-amd64  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 :

To xenbits.xen.org:/home/xen/git/osstest/ovmf.git
   c417c1b33d..8b8d6f8a3b  8b8d6f8a3beea391b1ec39ac347ef69501b44019 -> 
xen-tested-master

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

[Xen-devel] [linux-3.18 test] 133433: trouble: blocked/broken

[osstest service owner]

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

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-armhf-pvopsbroken
 build-arm64  broken
 build-armhf  broken
 build-arm64-xsm  broken
 build-amd64  broken
 build-i386-xsm   broken
 build-amd64-xsm  broken
 build-arm64-pvopsbroken
 build-amd64-pvopsbroken
 build-i386   broken
 build-i386-pvops broken
 build-i386-pvops  4 host-install(4)broken REGR. vs. 128858
 build-amd64   4 host-install(4)broken REGR. vs. 128858
 build-arm64-pvops 4 host-install(4)broken REGR. vs. 128858
 build-arm64   4 host-install(4)broken REGR. vs. 128858
 build-amd64-xsm   4 host-install(4)broken REGR. vs. 128858
 build-arm64-xsm   4 host-install(4)broken REGR. vs. 128858
 build-i3864 host-install(4)broken REGR. vs. 128858
 build-i386-xsm4 host-install(4)broken REGR. vs. 128858
 build-amd64-pvops 4 host-install(4)broken REGR. vs. 128858
 build-armhf-pvops 4 host-install(4)broken REGR. vs. 128858
 build-armhf   4 host-install(4)broken REGR. vs. 128858

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-examine  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ws16-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-i386-pvgrub  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-i386-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemut-ws16-amd64  1 build-check(1)  blocked n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-raw1 build-check(1)   blocked  n/a
 test-amd64-i386-examine   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-shadow1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-rtds  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-credit1   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-pvshim1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm  1 build-check(1)blocked n/a
 test-amd64-amd64-xl-qemut-debianhvm-amd64  1 build-check(1)blocked n/a
 test-amd64-i386-freebsd10-i386  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-win7-amd64  1 build-check(1)  blocked n/a
 test-armhf-armhf-xl-credit2   1 build-check(1)   blocked  n/a
 build-i386-rumprun1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-rtds  1 build-check(1)   blocked  n/a
 test-amd64-amd64-amd64-pvgrub  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked  n/a
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 build-armhf-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-xsm1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm  1 build-check(1)blocked n/a
 test-amd64-amd64-pygrub   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-win7-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-pair 1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-amd64-amd64-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemut-win10-i386  1 build-check(1)  blocked n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow  1 build-check(1) blocked n/a
 test-amd64-i386-libvirt-xsm   1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-amd64  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-win10-i386  1 build-check(1) blocked n/a
 test-armhf-armhf-xl-credit1   1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm  1 build-check(1) blocked 

[Xen-devel] [PATCH] xen-netback: fix occasional leak of grant ref mappings under memory pressure

[Igor Druzhinin]

Zero-copy callback flag is not yet set on frag list skb at the moment
xenvif_handle_frag_list() returns -ENOMEM. This eventually results in
leaking grant ref mappings since xenvif_zerocopy_callback() is never
called for these fragments. Those eventually build up and cause Xen
to kill Dom0 as the slots get reused for new mappings.

That behavior is observed under certain workloads where sudden spikes
of page cache usage for writes coexist with active atomic skb allocations.

Signed-off-by: Igor Druzhinin 
---
 drivers/net/xen-netback/netback.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/xen-netback/netback.c 
b/drivers/net/xen-netback/netback.c
index 80aae3a..2023317 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -1146,9 +1146,12 @@ static int xenvif_tx_submit(struct xenvif_queue *queue)
 
if (unlikely(skb_has_frag_list(skb))) {
if (xenvif_handle_frag_list(queue, skb)) {
+   struct sk_buff *nskb =
+   skb_shinfo(skb)->frag_list;
if (net_ratelimit())
netdev_err(queue->vif->dev,
   "Not enough memory to 
consolidate frag_list!\n");
+   xenvif_skb_zerocopy_prepare(queue, nskb);
xenvif_skb_zerocopy_prepare(queue, skb);
kfree_skb(skb);
continue;
-- 
2.7.4


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

[Xen-devel] [linux-4.19 test] 133431: regressions - trouble: blocked/broken/fail/pass

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-i386-pvops broken
 test-arm64-arm64-xl-credit1  broken
 build-i386-rumprun   broken
 build-amd64-xsm  broken
 build-arm64-libvirt  broken
 build-arm64-xsm  broken
 test-arm64-arm64-xl-credit2  broken
 test-arm64-arm64-xl  broken
 build-armhf  broken
 build-amd64  broken
 build-armhf-pvopsbroken
 build-i386-rumprun4 host-install(4)broken REGR. vs. 129313
 build-i386-pvops  4 host-install(4)broken REGR. vs. 129313
 build-amd64-xsm   4 host-install(4)broken REGR. vs. 129313
 build-arm64-libvirt   4 host-install(4)broken REGR. vs. 129313
 build-arm64-xsm   4 host-install(4)broken REGR. vs. 129313
 build-amd64   4 host-install(4)broken REGR. vs. 129313
 build-armhf   4 host-install(4)broken REGR. vs. 129313
 build-armhf-pvops 4 host-install(4)broken REGR. vs. 129313
 test-amd64-amd64-amd64-pvgrub broken in 133399
 test-armhf-armhf-xl-credit1  broken  in 133399
 test-amd64-amd64-xl-credit2  broken  in 133399
 test-amd64-i386-xl-qemuu-win10-i386   broken in 133399
 test-amd64-i386-xl-pvshimbroken  in 133399
 test-amd64-amd64-rumprun-amd64broken in 133399
 test-amd64-i386-xl-raw   broken  in 133399
 test-amd64-i386-qemut-rhel6hvm-amdbroken in 133399
 test-amd64-i386-freebsd10-i386broken in 133399
 test-armhf-armhf-xl-cubietruckbroken in 133399
 test-amd64-amd64-libvirt broken  in 133399
 test-amd64-amd64-libvirt-pair broken in 133399
 test-amd64-amd64-qemuu-nested-intel   broken in 133399
 test-amd64-i386-xl-qemuu-win10-i386 4 host-install(4) broken in 133399 REGR. 
vs. 129313
 test-amd64-i386-qemut-rhel6hvm-amd 4 host-install(4) broken in 133399 REGR. 
vs. 129313
 test-amd64-i386-xl-pvshim  4 host-install(4) broken in 133399 REGR. vs. 129313
 test-amd64-amd64-xl-credit2 4 host-install(4) broken in 133399 REGR. vs. 129313
 test-amd64-amd64-libvirt-pair 5 host-install/dst_host(5) broken in 133399 
REGR. vs. 129313
 test-amd64-amd64-rumprun-amd64 4 host-install(4) broken in 133399 REGR. vs. 
129313
 test-amd64-i386-xl-raw 4 host-install(4) broken in 133399 REGR. vs. 129313
 test-amd64-amd64-amd64-pvgrub 4 host-install(4) broken in 133399 REGR. vs. 
129313
 test-amd64-i386-examine   5 host-install broken in 133399 REGR. vs. 129313
 test-amd64-i386-freebsd10-i386 4 host-install(4) broken in 133399 REGR. vs. 
129313
 test-amd64-amd64-qemuu-nested-intel 4 host-install(4) broken in 133399 REGR. 
vs. 129313
 test-amd64-amd64-libvirt   4 host-install(4) broken in 133399 REGR. vs. 129313
 test-amd64-amd64-examine  5 host-install broken in 133399 REGR. vs. 129313
 test-armhf-armhf-xl-credit1 4 host-install(4) broken in 133399 REGR. vs. 129313
 test-armhf-armhf-xl-cubietruck 4 host-install(4) broken in 133399 REGR. vs. 
129313
 test-amd64-i386-libvirt   7 xen-boot   fail in 133399 REGR. vs. 129313
 test-amd64-i386-xl-shadow 7 xen-boot   fail in 133399 REGR. vs. 129313
 test-amd64-i386-libvirt-pair 11 xen-boot/dst_host fail in 133399 REGR. vs. 
129313
 test-amd64-i386-xl7 xen-boot   fail in 133399 REGR. vs. 129313
 test-amd64-i386-xl-qemuu-debianhvm-amd64 7 xen-boot fail in 133399 REGR. vs. 
129313
 test-amd64-amd64-xl-credit1   7 xen-boot   fail in 133399 REGR. vs. 129313
 test-amd64-amd64-i386-pvgrub  7 xen-boot   fail in 133399 REGR. vs. 129313
 test-amd64-amd64-xl-shadow7 xen-boot   fail in 133399 REGR. vs. 129313
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-boot fail in 
133399 REGR. vs. 129313
 test-amd64-i386-qemut-rhel6hvm-intel 7 xen-boot fail in 133399 REGR. vs. 129313
 test-amd64-amd64-xl-pvhv2-intel  7 xen-bootfail in 133399 REGR. vs. 129313
 test-amd64-amd64-xl-qcow2 10 debian-di-install fail in 133399 REGR. vs. 129313
 test-amd64-amd64-xl-qemut-debianhvm-amd64 10 debian-hvm-install fail in 133399 
REGR. vs. 129313
 test-amd64-amd64-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail in 133399 
REGR. vs. 129313
 test-armhf-armhf-xl-vhd   10 debian-di-install fail in 133399 REGR. vs. 129313

Tests which are failing intermittently (not blocking):
 test-arm64-arm64-xl   4 host-install(4)  broken 

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qemuu-win10-i386 broken
 test-amd64-amd64-xl  broken
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow broken
 test-amd64-i386-xl-qemut-win7-amd64 broken
 test-amd64-amd64-xl-xsm  broken
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm  broken
 test-amd64-i386-xl-qemuu-win10-i386 broken
 test-amd64-i386-freebsd10-amd64 broken
 test-amd64-i386-xl-raw   broken
 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm   broken
 test-amd64-amd64-qemuu-nested-amd broken
 test-amd64-i386-xl-qemut-debianhvm-amd64-xsmbroken
 test-amd64-i386-rumprun-i386 broken
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrictbroken
 test-amd64-amd64-xl-pvhv2-amd broken
 test-amd64-i386-pair broken
 test-amd64-i386-xl-qemuu-ws16-amd64 broken
 test-arm64-arm64-xl-xsm  broken
 test-arm64-arm64-xl-credit2  broken
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm   broken
 build-armhf  broken
 test-amd64-amd64-i386-pvgrub broken
 test-amd64-amd64-libvirt-xsm broken
 test-amd64-i386-xl-qemuu-win7-amd64 broken
 test-amd64-i386-qemut-rhel6hvm-intel broken
 test-amd64-i386-xl-qemut-win10-i386 broken
 test-amd64-i386-xl-shadowbroken
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm   broken
 build-armhf-pvopsbroken
 test-amd64-i386-xl-qemut-ws16-amd64 broken
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm   broken
 test-amd64-i386-qemuu-rhel6hvm-amd broken
 test-amd64-amd64-pairbroken
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict   broken
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsmbroken
 test-amd64-amd64-libvirt-vhd broken
 test-amd64-amd64-amd64-pvgrub broken
 test-amd64-i386-xl-qemuu-ovmf-amd64 broken
 test-amd64-i386-xl-xsm   broken
 test-amd64-i386-freebsd10-i386 broken
 test-amd64-i386-libvirt  broken
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsmbroken
 test-amd64-amd64-xl-qemuu-debianhvm-amd64   broken
 test-amd64-amd64-xl-qemuu-ws16-amd64 broken
 test-amd64-amd64-xl-qemut-win7-amd64 broken
 test-amd64-amd64-xl-qemuu-win7-amd64 broken
 test-amd64-i386-xl-qemut-debianhvm-amd64broken
 test-amd64-i386-xl-qemuu-debianhvm-amd64broken
 test-amd64-i386-qemuu-rhel6hvm-intel broken
 test-amd64-amd64-rumprun-amd64 broken
 test-amd64-amd64-pygrub  broken
 test-amd64-i386-qemut-rhel6hvm-amd broken
 test-amd64-amd64-qemuu-nested-intel broken
 test-amd64-amd64-xl-qemut-ws16-amd64 broken
 test-amd64-i386-xl-pvshimbroken
 test-amd64-i386-xl   broken
 test-amd64-i386-xl-qemuu-ws16-amd64  4 host-install(4) broken REGR. vs. 132748
 test-amd64-i386-qemut-rhel6hvm-amd  4 host-install(4)  broken REGR. vs. 132748
 test-amd64-i386-xl-pvshim 4 host-install(4)broken REGR. vs. 132748
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 4 host-install(4) broken 
REGR. vs. 132748
 test-amd64-i386-xl-qemuu-debianhvm-amd64 4 host-install(4) broken REGR. vs. 
132748
 test-amd64-i386-freebsd10-i386  4 host-install(4)  broken REGR. vs. 132748
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 4 host-install(4) broken REGR. 
vs. 132748
 test-amd64-i386-examine   5 host-install   broken REGR. vs. 132748
 test-amd64-i386-xl-qemut-debianhvm-amd64 4 host-install(4) broken REGR. vs. 
132748
 test-amd64-i386-rumprun-i386  4 host-install(4)broken REGR. vs. 132748
 test-amd64-i386-freebsd10-amd64  4 host-install(4) broken REGR. vs. 132748
 test-amd64-i386-xl-qemuu-win10-i386  4 host-install(4) broken REGR. vs. 132748
 test-amd64-i386-xl4 host-install(4)broken REGR. vs. 132748
 test-amd64-i386-qemut-rhel6hvm-intel 4 host-install(4) broken REGR. vs. 132748
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 4 host-install(4) broken 
REGR. vs. 132748
 test-amd64-amd64-qemuu-nested-intel  4 host-install(4) 

Re: [Xen-devel] [PATCH v10 2/6] xen: introduce DEFINE_SYMBOL

[Stefano Stabellini]

On Wed, 27 Feb 2019, Jan Beulich wrote:
> >>> On 26.02.19 at 19:43,  wrote:
> > On Tue, 26 Feb 2019, Ian Jackson wrote:
> >> Jan Beulich writes ("Re: [PATCH v10 2/6] xen: introduce DEFINE_SYMBOL"):
> >> > On 26.02.19 at 17:46,  wrote:
> >> > > I am not aware of a standard C type which could be used instead of
> >> > > this struct.  But I think you can use the `packed' attribute to get
> >> > > the right behaviour.  The GCC manual says:
> >> > > 
> >> > >  | Alignment can be decreased by specifying the 'packed' attribute.
> >> > >  | See below.
> >> ...
> >> > Until I've looked at this (again) now, I wasn't even aware that
> >> > one can combine packed and aligned attributes in a sensible
> >> > way. May I suggest that, because of this being a theoretical
> >> > issue only at this point, we limit ourselves to the build time
> >> > assertion you suggest?
> >> 
> >> I am not suggesting combining `packed' and `aligned'.  I am suggesting
> >> only `packed' (but based on text which is in the manual section for
> >> `aligned').  But I am happy with a build-time assertion if you don't
> >> want to add `packed'.  That is just as safe.
> > 
> > Could you please provide a rough example of the build-time assertion you
> > are thinking about? I am happy to add it.
> 
> BUILD_BUG_ON(alignof(*s1) != alignof(*s2));

Thanks! I noticed that BUILD_BUG_ON requires xen/lib.h which cannot be
included from xen/compiler.h. Actually, we were already erroneously
using BUILD_BUG_ON_ZERO in xen/compiler.h without including xen/lib.h.

My suggestion would be to move the definitions of BUG_ON, WARN_ON,
BUILD_BUG_ON and BUILD_BUG_ON_ZERO from xen/lib.h to compiler.h.
Everything works fine if I do that, and it seems a better fit.

Are you OK with this?

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

Re: [Xen-devel] [RFC PATCH 0/4] Add missing default labels to switch statements

[Stefano Stabellini]

On Wed, 27 Feb 2019, Lars Kurth wrote:
> > On 27 Feb 2019, at 10:16, Jan Beulich  wrote:
> > 
>  On 27.02.19 at 10:23,  wrote:
> >> 
> >> I recall that I read in an earlier thread that Julien and Stefano have 
> >> access to the document, which would leave Jan and a few members of Citrix 
> >> staff. Can those committers who need access raise their hands? I can then 
> >> go 
> >> ahead and order these.
> > 
> > Well, you've effectively raised my hand already. To be honest I'm not
> > sure I want it raised: I fear to break in tears when I would get to read
> > that book. In any event, I'd say ...
> 
> It's a reference document to look up stuff. Not something you would 
> necessarily read upfront.
> 
> >> Having followed this thread (and the other MISRA related one from 
> >> Stefano), 
> >> it seems to me that potentially each of these discussions is quite 
> >> divisive 
> >> and take up a lot of discussion and emotional energy. With 143 rules and 
> >> 16 
> >> "directives" (more like guidance) and some of the rules being mandatory 
> >> (73%) 
> >> and some advisory (27%), but the possibility to justify deviating from the 
> >> rule, maybe we are approaching this wrongly. 
> >> 
> >> I have some thoughts about the approach and will follow up on this thread 
> >> later today or tomorrow when I had some more time to clarify my thoughts.
> > 
> > ... don't order anything before we aren't clear whether we really want
> > to do this (or even any part thereof) to the code base.
> 
> Alright: firstly I need to explain that I asked EPAM to start looking a half 
> dozen or so "interesting" Misra compliance issues and post RFC patches. The 
> idea behind this was to gather data about how as a community we would handle 
> these  kind of issues. There was a discussion about Misra (or safety related 
> coding standards in general) at last years developer summit, which went 
> nowhere 
> due to lack of data. 
> 
> It is clear to me that as a community we have to deal with Misra C compliance 
> and other efforts to make Xen more easily safety certifiable seriously and 
> can't just wish it to go away. I think it is fair to say that the project is 
> facing increased competition from KVM and containers, while at the same time 
> Xen has unique advantages that lead vendors to go down the embedded/safety 
> route. If, as a community we just dismiss these efforts, we risk a fork or 
> those vendors going elsewhere. Neither would be good for the community.
> 
> Having seen the two discussions so far, it appears that even when we agree 
> that there is an issue, we seem to have real issues agreeing on workable 
> solutions. I also already had complaints that these threads generate to much 
> discussion (aka "noise").
> 
> What I don't know, is whether the two issues posted (this one and 
> https://markmail.org/message/ni3yziazuwb2aolx) are representative for the 
> kind 
> of issue we need to fix to achieve on Misra compliance, or whether they are 
> difficult outliers.
> 
> @Oleksandr: maybe you have some insights 
> 
> So the question is how we should approach this:
> 
> 1: One is to follow what we do now - post patches per issue and work through 
>them. This only really scales if the majority of patches are in essence
>uncontroversial.
> 
> 2: A slightly different approach would be for EPAM to post a few more 
> examples 
>of the type of issues that we would have to deal with if we want to be 
> MISRA
>compliant. But that we exercise restraint in the discussion knowing that 
> these
>are examples to inform a discussion at https://wiki.xenproject.org/wiki/
>Developer_Meeting/March2019_-_Safety_Certification and possible follow-up.
> 
>What I was after when I asked EPAM to post Misra related patches was to
>get a sense of the impact and a sense of how easily resolvable issues are.
>But I wouldn't expect a full resolution at this stage, if there
>is controversy. 
> 
>So maybe we can handle these in a different way. From my PoV, it would be 
> good 
>enough if key reviewers communicated per example whether
>- They accept that fixing the issue would be beneficial
>- What concerns they have
>- And how much they would fight for or against such a patch
>  (using the -2 ... +2 scale as outlined in "EXPRESSING AGREEMENT AND 
>  DISAGREEMENT" in https://xenproject.org/developers/governance/#decisions
>
>Clearly there can be some discussion, but we don't really need to "fight
>to the end" over these. 
> 
> 3: Or we could change approach completely and go for a more high-level
>design and/or analysis based approach before we do anything else. I will 
> expand 
>further down.
> 
> My personal preference would be to use 2 for a few patches, followed by 
> 3 as it gives us a different perspective.
> 
> Let me outline my thinking on 3:
> 
> There are a few things about Misra that we do not yet fully understand on a
> number of different 

[Xen-devel] [xen-unstable test] 133418: regressions - trouble: blocked/broken/fail/pass

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-xtf-amd64-amd64-3   broken
 test-amd64-amd64-xl-qemut-ws16-amd64 broken
 test-amd64-i386-xl-qemut-ws16-amd64 broken
 test-amd64-i386-xl-qemuu-win10-i386 broken
 test-xtf-amd64-amd64-5   broken
 build-armhf-pvopsbroken
 build-armhf  broken
 test-amd64-i386-xl-qemuu-ws16-amd64 broken
 test-amd64-amd64-xl-credit2  broken
 test-amd64-i386-libvirt-pair broken
 test-amd64-amd64-xl-qemuu-ovmf-amd64 broken
 test-amd64-amd64-livepatch   broken
 test-amd64-amd64-rumprun-amd64 broken
 test-amd64-i386-xl-qemut-ws16-amd64  4 host-install(4) broken REGR. vs. 133300
 test-amd64-amd64-rumprun-amd64  4 host-install(4)  broken REGR. vs. 133300
 test-amd64-amd64-xl-credit2   4 host-install(4)broken REGR. vs. 133300
 build-armhf   4 host-install(4)broken REGR. vs. 133300
 build-armhf-pvops 4 host-install(4)broken REGR. vs. 133300
 test-amd64-i386-xl   broken  in 133390
 test-amd64-amd64-xl-qemut-win10-i386  broken in 133390
 test-amd64-i386-qemuu-rhel6hvm-intel  broken in 133390
 test-armhf-armhf-xl-credit1  broken  in 133390
 test-amd64-amd64-xl-qemut-debianhvm-amd64 broken in 133390
 test-amd64-i386-xl-qemut-debianhvm-amd64  broken in 133390
 test-amd64-amd64-xl-qemuu-debianhvm-amd64 broken in 133390
 test-amd64-i386-freebsd10-i386broken in 133390
 test-amd64-i386-rumprun-i386 broken  in 133390
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm  broken in 133390
 test-armhf-armhf-xl-credit1 4 host-install(4) broken in 133390 REGR. vs. 133300
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 10 debian-hvm-install 
fail REGR. vs. 133300
 test-amd64-amd64-qemuu-nested-intel 10 debian-hvm-install fail REGR. vs. 133300
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail 
REGR. vs. 133300
 test-amd64-amd64-libvirt-vhd 10 debian-di-installfail REGR. vs. 133300
 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. 
vs. 133300
 test-amd64-amd64-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 
133300
 test-armhf-armhf-xl-arndale   7 xen-boot   fail in 133390 REGR. vs. 133300
 test-armhf-armhf-xl-vhd   10 debian-di-install fail in 133390 REGR. vs. 133300

Tests which are failing intermittently (not blocking):
 test-amd64-i386-qemuu-rhel6hvm-intel 4 host-install(4) broken in 133390 pass 
in 133418
 test-amd64-i386-rumprun-i386 4 host-install(4) broken in 133390 pass in 133418
 test-amd64-amd64-xl-qemut-win10-i386 4 host-install(4) broken in 133390 pass 
in 133418
 test-amd64-amd64-xl-qemut-debianhvm-amd64 4 host-install(4) broken in 133390 
pass in 133418
 test-amd64-i386-xl-qemut-debianhvm-amd64 4 host-install(4) broken in 133390 
pass in 133418
 test-amd64-amd64-xl-qemuu-debianhvm-amd64 4 host-install(4) broken in 133390 
pass in 133418
 test-amd64-i386-freebsd10-i386 4 host-install(4) broken in 133390 pass in 
133418
 test-amd64-i386-xl   4 host-install(4) broken in 133390 pass in 133418
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 4 host-install(4) broken in 
133390 pass in 133418
 test-amd64-i386-xl-qemuu-win10-i386  4 host-install(4)   broken pass in 133390
 test-amd64-i386-examine   5 host-install broken pass in 133390
 test-amd64-amd64-xl-qemut-ws16-amd64  4 host-install(4)  broken pass in 133390
 test-amd64-i386-libvirt-pair  5 host-install/dst_host(5) broken pass in 133390
 test-amd64-amd64-xl-qemuu-ovmf-amd64  4 host-install(4)  broken pass in 133390
 test-xtf-amd64-amd64-54 host-install(4)  broken pass in 133390
 test-xtf-amd64-amd64-34 host-install(4)  broken pass in 133390
 test-amd64-amd64-livepatch4 host-install(4)  broken pass in 133390
 test-amd64-amd64-examine  5 host-install broken pass in 133390
 test-amd64-amd64-qemuu-nested-amd 10 debian-hvm-install fail in 133390 pass in 
133418
 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 10 debian-hvm-install fail in 
133390 pass in 133418
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install 
fail in 133390 pass in 133418
 test-amd64-i386-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail in 133390 
pass in 133418
 test-amd64-amd64-i386-pvgrub 10 debian-di-install fail in 133390 pass in 133418
 test-amd64-amd64-xl-qcow2   10 debian-di-install fail in 133390 pass in 133418
 test-amd64-i386-xl-raw  

Re: [Xen-devel] [PATCH 1/6] xen: extend XEN_DOMCTL_memory_mapping to handle cacheability

[Julien Grall]

Hi Stefano,

On 2/26/19 11:07 PM, Stefano Stabellini wrote:

  struct xen_domctl_memory_mapping {
  uint64_aligned_t first_gfn; /* first page (hvm guest phys page) in range 
*/
  uint64_aligned_t first_mfn; /* first page (machine page) in range */
  uint64_aligned_t nr_mfns;   /* number of pages in range (>0) */
  uint32_t add_mapping;   /* add or remove mapping */
-uint32_t padding;   /* padding for 64-bit aligned structure */
+uint32_t cache_policy;  /* cacheability of the memory mapping */


Looking at this and the way you use it, the naming "cache" is quite 
confusing. On Arm, they are memory types (see B2.7 "Memory types and 
attributes" in DDI 0487D.a) and then you may have attribute such 
cachability attribute (write-through, write-back...) on top. The 
cacheability is also not applicable for "device memory".


"device memory" have other attributes related to gathering, re-ordering...

So a better naming would probably be "memory_policy".

Furthermore, those policies are only for configuring stage-2. The 
resulting memory type and attributes will be whatever is the strongest 
between stage-2 and stage-1 attributes. You can see the stage-2 
attributes as a way to give more or less freedom to the guest for 
configure the attributes.


For instance, by using p2m_mmio_direct_dev, the resulting attributes 
will always be Device-nGnRnE whatever how stage-1 has been configured.


In the case of p2m_mmio_direct_c (similar to p2m_ram_rw). The guest will 
be free to chose whatever pretty much any attributes (even Device-nGnRnE).


You might wonder why we didn't give more freedom to the guest from the 
start. One of the reason is it is quite unclear what are the consequence 
if you give that freedom to the guest. Whether there might be issues 
with the device when the attributes are not correct.


Furthermore, there are more handling required in the hypervisor as if 
the memory can be cached, you will need to clear the cache in order to 
prevent leakage to another domain if the mappings get reassigned.


For completeness, I should mention the feature S2FWB present in ARMv8.4 
and onwards. From my understanding, this could be used to force 
resulting memory type. I am not suggesting to implement it now, but we 
should keep it in my mind while writing the interface exposed in libxl.


To summarize, if we go ahead, we should try to make the documentation 
more clearer on what each policy means and the implications on the 
guest. I think we should also mark this a not security supported because 
it the unknown interactions with devices.


Cheers,

--
Julien Grall

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

[Xen-devel] [libvirt test] 133428: regressions - trouble: blocked/broken/fail/pass

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-armhf  broken
 build-armhf-pvopsbroken
 test-amd64-amd64-libvirt-pair broken
 test-amd64-amd64-libvirt broken
 test-amd64-amd64-libvirt-pair 5 host-install/dst_host(5) broken REGR. vs. 
133272
 test-amd64-amd64-libvirt  4 host-install(4)broken REGR. vs. 133272
 build-armhf-pvops 4 host-install(4)broken REGR. vs. 133272
 build-armhf   4 host-install(4)broken REGR. vs. 133272
 test-amd64-i386-libvirt-xsm  12 guest-start  fail REGR. vs. 133272
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 18 guest-destroy fail REGR. 
vs. 133272

Tests which did not succeed, but are not blocking:
 build-armhf-libvirt   1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt 14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-qcow2 12 migrate-support-checkfail never pass
 test-arm64-arm64-libvirt-qcow2 13 saverestore-support-checkfail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass

version targeted for testing:
 libvirt  ac62e297dba01acd7aebe3f379a63b32ab617bfd
baseline version:
 libvirt  0624ac3fa846b3e2a8e70e4cc2fd8477710cd76d

Last test of basis   133272  2019-02-15 22:58:38 Z   11 days
Failing since133306  2019-02-19 04:19:06 Z8 days6 attempts
Testing same since   133428  2019-02-25 18:45:29 Z2 days1 attempts


People who touched revisions under test:
  Andrea Bolognani 
  Chris Venteicher 
  Christian Ehrhardt 
  Daniel P. Berrangé 
  David Kiarie 
  Eric Blake 
  Jamie Strandboge 
  Jiri Denemark 
  John Ferlan 
  Julio Faracco 
  Ján Tomko 
  Laine Stump 
  Marc Hartmayer 
  Michal Privoznik 
  Nikolay Shirokovskiy 
  Peter Krempa 
  Roman Bogorodskiy 

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-arm64  pass
 build-armhf  broken  
 build-i386   pass
 build-amd64-libvirt  pass
 build-arm64-libvirt  pass
 build-armhf-libvirt  blocked 
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-arm64-pvopspass
 build-armhf-pvopsbroken  
 build-i386-pvops pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmfail
 test-amd64-amd64-libvirt-xsm pass
 test-arm64-arm64-libvirt-xsm pass
 test-amd64-i386-libvirt-xsm  fail
 test-amd64-amd64-libvirt broken  
 test-arm64-arm64-libvirt pass
 test-armhf-armhf-libvirt blocked 
 test-amd64-i386-libvirt  pass
 test-amd64-amd64-libvirt-pairbroken  
 test-amd64-i386-libvirt-pair pass
 test-arm64-arm64-libvirt-qcow2   pass
 test-armhf-armhf-libvirt-raw blocked 
 test-amd64-amd64-libvirt-vhd pass

Re: [Xen-devel] XEN on R-CAR H3

[Oleksandr]

Hi, Amit




BTW, we use mkimage tool to create Xen image to be loaded:

mkimage -A arm64 -C none -T kernel -a 0x7808 -e 0x7808 -n 
"XEN" xen/xen xen-uImage



I am sorry, I had missed "-d" key before xen image.

The proper command is:

mkimage -A arm64 -C none -T kernel -a 0x7808 -e 0x7808 -n "XEN" 
-d xen/xen xen-uImage



--
Regards,

Oleksandr Tyshchenko


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

Re: [Xen-devel] [PATCH 3/6] libxl/xl: add cacheability option to iomem

[Julien Grall]

Hi Stefano,

On 2/26/19 11:07 PM, Stefano Stabellini wrote:

Parse a new cacheability option for the iomem parameter, it can be
"devmem" for device memory mappings, which is the default, or "memory"
for normal memory mappings.

Store the parameter in a new field in libxl_iomem_range.

Pass the cacheability option to xc_domain_memory_mapping.

Signed-off-by: Stefano Stabellini 
CC: ian.jack...@eu.citrix.com
CC: wei.l...@citrix.com
---
  docs/man/xl.cfg.pod.5.in|  4 +++-
  tools/libxl/libxl_create.c  | 12 +++-
  tools/libxl/libxl_types.idl |  7 +++


Extension of the libxl_types.idl should be companied with a new define 
in libxl.h. So a toolstack can deal with multiple libxl version.



  tools/xl/xl_parse.c | 17 +++--
  4 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in
index b1c0be1..655008a 100644
--- a/docs/man/xl.cfg.pod.5.in
+++ b/docs/man/xl.cfg.pod.5.in
@@ -1222,7 +1222,7 @@ is given in hexadecimal format and may either be a range, e.g. 
C<2f8-2ff>
  It is recommended to only use this option for trusted VMs under
  administrator's control.
  
-=item B

+=item B


Below you suggest the cacheability is option. However, the is not 
reflected here. I think you want to put ',CACHEABILITY' between [] as it 
is done for '@GFN'.


  
  Allow auto-translated domains to access specific hardware I/O memory pages.
  
@@ -1233,6 +1233,8 @@ B is not specified, the mapping will be performed using B

  as a start in the guest's address space, therefore performing a 1:1 mapping
  by default.
  All of these values must be given in hexadecimal format.
+B can be "devmem" for device memory, the default if not
+specified, or it can be "memory" for normal memory.


I was planning to comment about the naming and documentation. But I will 
do it in patch #1 where Jan already started a discussion about it.



diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index 352cd21..1da2670 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -1883,6 +1883,7 @@ void parse_config_data(const char *config_source,
  }
  for (i = 0; i < num_iomem; i++) {
  int used;
+char cache[7];
  
  buf = xlu_cfg_get_listitem (iomem, i);

  if (!buf) {
@@ -1891,15 +1892,27 @@ void parse_config_data(const char *config_source,
  exit(1);
  }
  libxl_iomem_range_init(_info->iomem[i]);
-ret = sscanf(buf, "%" SCNx64",%" SCNx64"%n@%" SCNx64"%n",
+ret = sscanf(buf, "%" SCNx64",%" SCNx64"%n@%" SCNx64"%n,%6s%n",
   _info->iomem[i].start,
   _info->iomem[i].number, ,
- _info->iomem[i].gfn, );
+ _info->iomem[i].gfn, ,
+ cache, );


If I read it correctly, you will require the GFN to be specified in 
order to get the "cacheability". Am I correct?


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH 1/6] xen: extend XEN_DOMCTL_memory_mapping to handle cacheability

[Julien Grall]

Hi Stefano,

On 2/26/19 11:07 PM, Stefano Stabellini wrote:

Reuse the existing padding field to pass cacheability information about
the memory mapping, specifically, whether the memory should be mapped as
normal memory or as device memory (this is what we have today).

Add a cacheability parameter to map_mmio_regions. 0 means device
memory, which is what we have today.

On ARM, map device memory as p2m_mmio_direct_dev (as it is already done
today) and normal memory as p2m_ram_rw.

On x86, return error if the cacheability requested is not device memory.

Signed-off-by: Stefano Stabellini 
CC: jbeul...@suse.com
CC: andrew.coop...@citrix.com
---
  xen/arch/arm/gic-v2.c|  3 ++-
  xen/arch/arm/p2m.c   | 19 +--
  xen/arch/arm/platforms/exynos5.c |  4 ++--
  xen/arch/arm/platforms/omap5.c   |  8 
  xen/arch/arm/vgic-v2.c   |  2 +-
  xen/arch/arm/vgic/vgic-v2.c  |  2 +-
  xen/arch/x86/hvm/dom0_build.c|  7 +--
  xen/arch/x86/mm/p2m.c|  6 +-
  xen/common/domctl.c  |  8 +---
  xen/drivers/vpci/header.c|  3 ++-
  xen/include/public/domctl.h  |  4 +++-
  xen/include/xen/p2m-common.h |  3 ++-
  12 files changed, 49 insertions(+), 20 deletions(-)

diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index e7eb01f..1ea3da2 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -690,7 +690,8 @@ static int gicv2_map_hwdown_extra_mappings(struct domain *d)
  
  ret = map_mmio_regions(d, gaddr_to_gfn(v2m_data->addr),

 PFN_UP(v2m_data->size),
-   maddr_to_mfn(v2m_data->addr));
+   maddr_to_mfn(v2m_data->addr),
+   CACHEABILITY_DEVMEM);
  if ( ret )
  {
  printk(XENLOG_ERR "GICv2: Map v2m frame to d%d failed.\n",
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 30cfb01..5b8fcc5 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1068,9 +1068,24 @@ int unmap_regions_p2mt(struct domain *d,
  int map_mmio_regions(struct domain *d,
   gfn_t start_gfn,
   unsigned long nr,
- mfn_t mfn)
+ mfn_t mfn,
+ uint32_t cache_policy)


Rather than extending map_mmio_regions, I would prefer if we kill this 
function (and unmap_mmio_mmio_regions) and instead use map_regions_p2mt.


This means the conversation to p2mt should be done in the DOMCTL handling.


  {
-return p2m_insert_mapping(d, start_gfn, nr, mfn, p2m_mmio_direct_dev);
+p2m_type_t t;
+
+switch ( cache_policy )
+{
+case CACHEABILITY_MEMORY:
+t = p2m_ram_rw;
+break;
+case CACHEABILITY_DEVMEM:
+t = p2m_mmio_direct_dev;
+break;
+default:
+return -ENOSYS;


We tend to use EOPNOTSUPP in such a case.

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH] drm: add func to better detect wether swiotlb is needed

[Konrad Rzeszutek Wilk]

.snip..
> > -u64 drm_get_max_iomem(void)
> > +bool drm_need_swiotlb(int dma_bits)
> >  {
> > struct resource *tmp;
> > resource_size_t max_iomem = 0;
> > 
> > +   /*
> > +* Xen paravirtual hosts require swiotlb regardless of requested dma
> > +* transfer size.
> > +*
> > +* NOTE: Really, what it requires is use of the dma_alloc_coherent
> > +*   allocator used in ttm_dma_populate() instead of
> > +*   ttm_populate_and_map_pages(), which bounce buffers so much
> > in
> > +*   Xen it leads to swiotlb buffer exhaustion.
> > +*/
> > +   if (xen_pv_domain())
> 
> I've not been following all of the ins and outs of the discussion on this so 
> apologies if I'm missing some context, but...
> 
> This looks like the wrong test to me. I think it should be:
> 
> if ( xen_swiotlb )

Ah, that could be as well. 
> 

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

Re: [Xen-devel] [RESEND PATCH 0/7] Add FOLL_LONGTERM to GUP fast and use it

[Ira Weiny]

On Tue, Feb 19, 2019 at 09:30:33PM -0800, 'Ira Weiny' wrote:
> From: Ira Weiny 
> 
> Resending these as I had only 1 minor comment which I believe we have covered
> in this series.  I was anticipating these going through the mm tree as they
> depend on a cleanup patch there and the IB changes are very minor.  But they
> could just as well go through the IB tree.
> 
> NOTE: This series depends on my clean up patch to remove the write parameter
> from gup_fast_permitted()[1]
> 
> HFI1, qib, and mthca, use get_user_pages_fast() due to it performance
> advantages.  These pages can be held for a significant time.  But
> get_user_pages_fast() does not protect against mapping of FS DAX pages.
> 
> Introduce FOLL_LONGTERM and use this flag in get_user_pages_fast() which
> retains the performance while also adding the FS DAX checks.  XDP has also
> shown interest in using this functionality.[2]
> 
> In addition we change get_user_pages() to use the new FOLL_LONGTERM flag and
> remove the specialized get_user_pages_longterm call.
> 
> [1] https://lkml.org/lkml/2019/2/11/237
> [2] https://lkml.org/lkml/2019/2/11/1789

Is there anything I need to do on this series or does anyone have any
objections to it going into 5.1?  And if so who's tree is it going to go
through?

Thanks,
Ira

> 
> Ira Weiny (7):
>   mm/gup: Replace get_user_pages_longterm() with FOLL_LONGTERM
>   mm/gup: Change write parameter to flags in fast walk
>   mm/gup: Change GUP fast to use flags rather than a write 'bool'
>   mm/gup: Add FOLL_LONGTERM capability to GUP fast
>   IB/hfi1: Use the new FOLL_LONGTERM flag to get_user_pages_fast()
>   IB/qib: Use the new FOLL_LONGTERM flag to get_user_pages_fast()
>   IB/mthca: Use the new FOLL_LONGTERM flag to get_user_pages_fast()
> 
>  arch/mips/mm/gup.c  |  11 +-
>  arch/powerpc/kvm/book3s_64_mmu_hv.c |   4 +-
>  arch/powerpc/kvm/e500_mmu.c |   2 +-
>  arch/powerpc/mm/mmu_context_iommu.c |   4 +-
>  arch/s390/kvm/interrupt.c   |   2 +-
>  arch/s390/mm/gup.c  |  12 +-
>  arch/sh/mm/gup.c|  11 +-
>  arch/sparc/mm/gup.c |   9 +-
>  arch/x86/kvm/paging_tmpl.h  |   2 +-
>  arch/x86/kvm/svm.c  |   2 +-
>  drivers/fpga/dfl-afu-dma-region.c   |   2 +-
>  drivers/gpu/drm/via/via_dmablit.c   |   3 +-
>  drivers/infiniband/core/umem.c  |   5 +-
>  drivers/infiniband/hw/hfi1/user_pages.c |   5 +-
>  drivers/infiniband/hw/mthca/mthca_memfree.c |   3 +-
>  drivers/infiniband/hw/qib/qib_user_pages.c  |   8 +-
>  drivers/infiniband/hw/qib/qib_user_sdma.c   |   2 +-
>  drivers/infiniband/hw/usnic/usnic_uiom.c|   9 +-
>  drivers/media/v4l2-core/videobuf-dma-sg.c   |   6 +-
>  drivers/misc/genwqe/card_utils.c|   2 +-
>  drivers/misc/vmw_vmci/vmci_host.c   |   2 +-
>  drivers/misc/vmw_vmci/vmci_queue_pair.c |   6 +-
>  drivers/platform/goldfish/goldfish_pipe.c   |   3 +-
>  drivers/rapidio/devices/rio_mport_cdev.c|   4 +-
>  drivers/sbus/char/oradax.c  |   2 +-
>  drivers/scsi/st.c   |   3 +-
>  drivers/staging/gasket/gasket_page_table.c  |   4 +-
>  drivers/tee/tee_shm.c   |   2 +-
>  drivers/vfio/vfio_iommu_spapr_tce.c |   3 +-
>  drivers/vfio/vfio_iommu_type1.c |   3 +-
>  drivers/vhost/vhost.c   |   2 +-
>  drivers/video/fbdev/pvr2fb.c|   2 +-
>  drivers/virt/fsl_hypervisor.c   |   2 +-
>  drivers/xen/gntdev.c|   2 +-
>  fs/orangefs/orangefs-bufmap.c   |   2 +-
>  include/linux/mm.h  |  17 +-
>  kernel/futex.c  |   2 +-
>  lib/iov_iter.c  |   7 +-
>  mm/gup.c| 220 
>  mm/gup_benchmark.c  |   5 +-
>  mm/util.c   |   8 +-
>  net/ceph/pagevec.c  |   2 +-
>  net/rds/info.c  |   2 +-
>  net/rds/rdma.c  |   3 +-
>  44 files changed, 232 insertions(+), 180 deletions(-)
> 
> -- 
> 2.20.1
> 

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

Re: [Xen-devel] [PATCH for-4.12 RFC] xen/console: Handle NUL character in buffer sent via CONSOLEIO_write

[Julien Grall]

Hi Wei,

On 2/27/19 12:55 PM, Wei Liu wrote:

On Tue, Feb 26, 2019 at 11:03:51PM +, Julien Grall wrote:

After upgrading Debian to Buster, I started noticing console mangling
when using zsh. This is happenning because output sent by zsh to the
console may contain NUL character in the middle of the buffer.

Linux is sending the buffer as it is to Xen console via CONSOLEIO_write.
However, the implementation in Xen considers NUL character is used to
terminate the buffer and therefore will ignore anything after it.

The actual documentation of CONSOLEIO_write is pretty limited. From the
declaration, the hypercall takes a buffer and size. So this could lead
to think the NUL character is allowed in the middle of the buffer.

This patch updates the console API to pass the size along the buffer
down so we can remove the reliance on buffer terminating by a NUL
character.

Signed-off-by: Julien Grall 

---


[...]

@@ -527,7 +527,7 @@ static inline void xen_console_write_debug_port(const char 
*buf, size_t len)
  static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer, int 
count)
  {
  char kbuf[128];
-int kcount = 0;
+unsigned int kcount = 0;
  struct domain *cd = current->domain;
  
  while ( count > 0 )

@@ -547,8 +547,8 @@ static long 
guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer, int count)
  /* Use direct console output as it could be interactive */
  spin_lock_irq(_lock);
  
-sercon_puts(kbuf);

-video_puts(kbuf);
+sercon_puts(kbuf, kcount);
+video_puts(kbuf, kcount);
  


I think you missed the non-hwdom branch in the same function. It still
strips non-printable characters.


Good point. The non-printable characters was added by Daniel in commit 
48d50de8e0 " console: buffer and show origin of guest PV writes" without 
much explanation.


The only reason I can see is, as we buffer the guest writes, the console 
would be screwed if we split an escape sequence. Furthermore, for guest 
output, we will always append "(dX)" to the output. So I am not entirely 
sure what to do in the non-hwdom case.


Any opinions?




  int console_suspend(void)

[...]

diff --git a/xen/drivers/char/consoled.c b/xen/drivers/char/consoled.c
index 552abf5766..3e849a2557 100644
--- a/xen/drivers/char/consoled.c
+++ b/xen/drivers/char/consoled.c
@@ -77,7 +77,7 @@ size_t consoled_guest_rx(void)
  
  if ( idx >= BUF_SZ )

  {
-pv_console_puts(buf);
+pv_console_puts(buf, BUF_SZ);
  idx = 0;
  }
  }
@@ -85,7 +85,7 @@ size_t consoled_guest_rx(void)
  if ( idx )
  {
  buf[idx] = '\0';


Can this be deleted? Now that you've explicitly sized the buffer.


We probably can delete a few of the '\0' over the console code. I will 
have a look at it.


Cheers,

--
Julien Grall

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

[Xen-devel] [ovmf test] 133445: trouble: blocked/broken

[osstest service owner]

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

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64-pvopsbroken
 build-i386-pvops broken
 build-i386   broken
 build-i386-xsm   broken
 build-amd64-xsm  broken
 build-amd64  broken
 build-i3864 host-install(4)broken REGR. vs. 133291
 build-amd64-xsm   4 host-install(4)broken REGR. vs. 133291
 build-amd64-pvops 4 host-install(4)broken REGR. vs. 133291
 build-amd64   4 host-install(4)broken REGR. vs. 133291
 build-i386-xsm4 host-install(4)broken REGR. vs. 133291
 build-i386-pvops  4 host-install(4)broken REGR. vs. 133291

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
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a

version targeted for testing:
 ovmf 7d180efeaa03df25973416dc0aad099f4fe7e251
baseline version:
 ovmf c417c1b33d06ef6ae96adb373201a5a3c3b38772

Last test of basis   133291  2019-02-18 01:41:15 Z9 days
Failing since133305  2019-02-19 00:41:21 Z8 days9 attempts
Testing same since   133445  2019-02-26 19:39:44 Z0 days1 attempts


People who touched revisions under test:
  Albecki Mateusz 
  Albecki, Mateusz 
  Anthony PERARD 
  Ard Biesheuvel 
  Ashish Singhal 
  Bob Feng 
  Chasel Chiu 
  Chasel, Chiu 
  Chen A Chen 
  Dandan Bi 
  Edgar Handal 
  Fan, ZhijuX 
  Feng, Bob C 
  Gonzalez Del Cueto, Rodrigo 
  Hao Wu 
  Jeff Brasen 
  Jian J Wang 
  Jiaxin Wu 
  Jiewen Yao 
  Jordan Justen 
  Julien Grall 
  Laszlo Ersek 
  Liming Gao 
  Max Knutsen 
  Pete Batard 
  Ray Ni 
  Rodrigo Gonzalez del Cueto 
  Ruiyu Ni 
  Sami Mujawar 
  Shenglei Zhang 
  Star Zeng 
  Wu Jiaxin 
  Zhichao Gao 
  Zhiju.Fan 

jobs:
 build-amd64-xsm  broken  
 build-i386-xsm   broken  
 build-amd64  broken  
 build-i386   broken  
 build-amd64-libvirt  blocked 
 build-i386-libvirt   blocked 
 build-amd64-pvopsbroken  
 build-i386-pvops broken  
 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-job build-i386-pvops broken
broken-job build-i386 broken
broken-job build-i386-xsm broken
broken-job build-amd64-xsm broken
broken-job build-amd64 broken
broken-step build-i386 host-install(4)
broken-step build-amd64-xsm host-install(4)
broken-step build-amd64-pvops host-install(4)
broken-step build-amd64 host-install(4)
broken-step build-i386-xsm host-install(4)
broken-step build-i386-pvops host-install(4)

Not pushing.

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

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

[Xen-devel] [linux-next test] 133420: regressions - trouble: blocked/broken/fail/pass

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-libvirt-vhd broken
 test-amd64-amd64-xl-pvhv2-amd broken
 test-amd64-amd64-xl-qemuu-ws16-amd64 broken
 test-amd64-amd64-amd64-pvgrub broken
 build-armhf-pvopsbroken
 build-armhf  broken
 test-amd64-amd64-xl-pvhv2-amd  4 host-install(4)   broken REGR. vs. 133373
 test-amd64-amd64-libvirt-vhd  4 host-install(4)broken REGR. vs. 133373
 test-amd64-amd64-xl-qemuu-ws16-amd64 4 host-install(4) broken REGR. vs. 133373
 test-amd64-amd64-amd64-pvgrub  4 host-install(4)   broken REGR. vs. 133373
 build-armhf   4 host-install(4)broken REGR. vs. 133373
 build-armhf-pvops 4 host-install(4)broken REGR. vs. 133373
 test-arm64-arm64-xl-xsm   7 xen-boot fail REGR. vs. 133373
 test-arm64-arm64-xl  12 guest-start  fail REGR. vs. 133373
 test-arm64-arm64-xl-credit1  12 guest-start  fail REGR. vs. 133373
 test-arm64-arm64-libvirt-xsm 12 guest-start  fail REGR. vs. 133373
 test-amd64-amd64-xl-shadow7 xen-boot fail REGR. vs. 133373
 test-arm64-arm64-xl-credit2  12 guest-start  fail REGR. vs. 133373
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install 
fail REGR. vs. 133373
 test-amd64-amd64-examine  4 memdisk-try-append   fail REGR. vs. 133373
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 10 debian-hvm-install fail 
REGR. vs. 133373
 build-i3866 xen-buildfail REGR. vs. 133373
 build-i386-xsm6 xen-buildfail REGR. vs. 133373

Tests which did not succeed, but are not blocking:
 test-amd64-i386-examine   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-cubietruck  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemut-ws16-amd64  1 build-check(1)  blocked n/a
 test-armhf-armhf-xl-credit1   1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemut-win7-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-qemuu-rhel6hvm-intel  1 build-check(1) blocked n/a
 test-amd64-i386-qemut-rhel6hvm-amd  1 build-check(1)   blocked n/a
 test-amd64-i386-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-shadow 1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-vhd   1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-win7-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-debianhvm-amd64  1 build-check(1) blocked n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm  1 build-check(1) blocked n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 build-i386-rumprun1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-raw1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-i386  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 1 build-check(1) blocked 
n/a
 test-amd64-i386-libvirt   1 build-check(1)   blocked  n/a
 build-armhf-libvirt   1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-credit2   1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-xsm1 build-check(1)   blocked  n/a
 test-amd64-i386-xl1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-qemuu-rhel6hvm-amd  1 build-check(1)   blocked n/a
 test-amd64-i386-freebsd10-amd64  1 build-check(1)   blocked  n/a
 test-armhf-armhf-examine  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked 
n/a
 test-armhf-armhf-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemut-debianhvm-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-win10-i386  1 build-check(1)  blocked n/a
 test-armhf-armhf-xl-rtds  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow  1 

Re: [Xen-devel] [PATCH 1/3] mwait-idle: add support for using halt

[Woods, Brian]

On 2/27/19 7:47 AM, Wei Liu wrote:
> On Mon, Feb 25, 2019 at 08:23:58PM +, Woods, Brian wrote:
>> Some AMD processors can use a mixture of mwait and halt for accessing
>> various c-states.  In preparation for adding support for AMD processors,
>> update the mwait-idle driver to optionally use halt.
>>
>> Signed-off-by: Brian Woods 
>> ---
>>   xen/arch/x86/cpu/mwait-idle.c | 40 +---
>>   1 file changed, 33 insertions(+), 7 deletions(-)
>>
>> diff --git a/xen/arch/x86/cpu/mwait-idle.c b/xen/arch/x86/cpu/mwait-idle.c
>> index f89c52f256..a063e39d60 100644
>> --- a/xen/arch/x86/cpu/mwait-idle.c
>> +++ b/xen/arch/x86/cpu/mwait-idle.c
>> @@ -103,6 +103,11 @@ static const struct cpuidle_state {
>>   
>>   #define CPUIDLE_FLAG_DISABLED  0x1
>>   /*
>> + * On certain AMD families that support mwait, only c1 can be reached by
>> + * mwait and to reach c2, halt has to be used.
>> + */
>> +#define CPUIDLE_FLAG_USE_HALT   0x2
>> +/*
>>* Set this flag for states where the HW flushes the TLB for us
>>* and so we don't need cross-calls to keep it consistent.
>>* If this flag is set, SW flushes the TLB, so even if the
>> @@ -783,8 +788,23 @@ static void mwait_idle(void)
>>   
>>  update_last_cx_stat(power, cx, before);
>>   
>> -if (cpu_is_haltable(cpu))
>> -mwait_idle_with_hints(eax, MWAIT_ECX_INTERRUPT_BREAK);
>> +if (cpu_is_haltable(cpu)) {
>> +struct cpu_info *info;
>> +switch (cx->entry_method) {
>> +case ACPI_CSTATE_EM_FFH:
>> +mwait_idle_with_hints(eax, MWAIT_ECX_INTERRUPT_BREAK);
>> +break;
>> +case ACPI_CSTATE_EM_HALT:
> 
>> +info = get_cpu_info();
>> +spec_ctrl_enter_idle(info);
>> +safe_halt();
>> +spec_ctrl_exit_idle(info);
> 
> May I suggest you make this snippet a function? The same code snippet
> appears a few lines above.
> 
> Wei.
> 
It's used in various other places as well (cpu_idle.c, x86/domain.c), 
would a function like:

void safe_halt_with_spec(cpu_info *info)
{
 if (!info)
 info = get_cpu_info();

 spec_ctrl_enter_idle(info);
 safe_halt();
 spec_ctrl_exit_idle(info);
}

work since that way it could be used in other places where info is 
already defined?
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Preparing for Xen Project GSoC applications : Deadline Feb 6

[Lars Kurth]

Hi all,

looking through the list of GSoC orgs, the following orgs that sometimes have 
Xen related projects have been accepted into GSoC this year:
* https://wiki.freebsd.org/SummerOfCodeIdeas
* https://gsoc.honeynet.org/
* https://elinux.org/BeagleBoard/GSoC/Ideas [there is a Xen specific proposal 
not yet on the table]

If you come across anyone asking us, please point them in the right direction

Best Regards
Lars


> On 25 Feb 2019, at 18:36, Lars Kurth  wrote:
> 
> Hi all:
> 
> a quick note the we have *not* been accepted into GSoC this year (the current 
> acceptance rate seems to be that projects get GSOC slots every 3 years). A 
> big thank you to those who contributed to the project list. 
> 
> This does not mean that there won't be Xen related projects. There are a 
> number of Xen related projects in a number of other communities. Once the 
> full list of accepted organisations is available, I will collate a list and 
> get back to you
> 
> Best Regards
> Lars
> 
>> On 15 Jan 2019, at 13:33, Lars Kurth  wrote:
>> 
>> Hi all, 
>> 
>> I will be applying as a mentoring organisation for GSoC again this year: the 
>> application deadline is Feb 6 and by then we need to have 
>> https://wiki.xenproject.org/wiki/Outreach_Program_Projects in order. Given 
>> that we didn't get in last year, there is a 50/50 chance we get in this year.
>> 
>> Everyone on the CC list has projects listed on 
>> https://wiki.xenproject.org/wiki/Outreach_Program_Projects
>> 
>> Our project list is a little old and stale and that shows: we do need to 
>> bring this up-to-date and freshen it up with new projects. I believe that 
>> the Mini-OS and Unikraft projects need looking at. And we may have some new 
>> sensible projects in the Hypervisor itself. Mindy already agreed to go over 
>> the Mirage OS list.
>> 
>> If you want to withdraw your project: please let me know and I delete it: 
>> but let me know WHY you want to withdraw. E.g. is it complete
>> 
>> @Doug, @Comitters
>> Re 
>> https://wiki.xenproject.org/wiki/Outreach_Program_Projects#Code_Standards_Checking_using_clang-format
>> Given that there has been some work on clang-format by EPAM, which no-one 
>> has looked at I am tempted to throw this out or re-do the project. Aka, die 
>> a next phase which includes integrating the tool into our workflow. But that 
>> may be too hard
>> Any views?
>> 
>> Regards
>> Lars
> 


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

Re: [Xen-devel] [RFC PATCH 0/4] Add missing default labels to switch statements

[Lars Kurth]

> On 27 Feb 2019, at 10:16, Jan Beulich  wrote:
> 
 On 27.02.19 at 10:23,  wrote:
>> 
>> I recall that I read in an earlier thread that Julien and Stefano have 
>> access to the document, which would leave Jan and a few members of Citrix 
>> staff. Can those committers who need access raise their hands? I can then go 
>> ahead and order these.
> 
> Well, you've effectively raised my hand already. To be honest I'm not
> sure I want it raised: I fear to break in tears when I would get to read
> that book. In any event, I'd say ...

It's a reference document to look up stuff. Not something you would 
necessarily read upfront.

>> Having followed this thread (and the other MISRA related one from Stefano), 
>> it seems to me that potentially each of these discussions is quite divisive 
>> and take up a lot of discussion and emotional energy. With 143 rules and 16 
>> "directives" (more like guidance) and some of the rules being mandatory 
>> (73%) 
>> and some advisory (27%), but the possibility to justify deviating from the 
>> rule, maybe we are approaching this wrongly. 
>> 
>> I have some thoughts about the approach and will follow up on this thread 
>> later today or tomorrow when I had some more time to clarify my thoughts.
> 
> ... don't order anything before we aren't clear whether we really want
> to do this (or even any part thereof) to the code base.

Alright: firstly I need to explain that I asked EPAM to start looking a half 
dozen or so "interesting" Misra compliance issues and post RFC patches. The 
idea behind this was to gather data about how as a community we would handle 
these  kind of issues. There was a discussion about Misra (or safety related 
coding standards in general) at last years developer summit, which went nowhere 
due to lack of data. 

It is clear to me that as a community we have to deal with Misra C compliance 
and other efforts to make Xen more easily safety certifiable seriously and 
can't just wish it to go away. I think it is fair to say that the project is 
facing increased competition from KVM and containers, while at the same time 
Xen has unique advantages that lead vendors to go down the embedded/safety 
route. If, as a community we just dismiss these efforts, we risk a fork or 
those vendors going elsewhere. Neither would be good for the community.

Having seen the two discussions so far, it appears that even when we agree 
that there is an issue, we seem to have real issues agreeing on workable 
solutions. I also already had complaints that these threads generate to much 
discussion (aka "noise").

What I don't know, is whether the two issues posted (this one and 
https://markmail.org/message/ni3yziazuwb2aolx) are representative for the kind 
of issue we need to fix to achieve on Misra compliance, or whether they are 
difficult outliers.

@Oleksandr: maybe you have some insights 

So the question is how we should approach this:

1: One is to follow what we do now - post patches per issue and work through 
   them. This only really scales if the majority of patches are in essence
   uncontroversial.

2: A slightly different approach would be for EPAM to post a few more examples 
   of the type of issues that we would have to deal with if we want to be MISRA
   compliant. But that we exercise restraint in the discussion knowing that 
these
   are examples to inform a discussion at https://wiki.xenproject.org/wiki/
   Developer_Meeting/March2019_-_Safety_Certification and possible follow-up.

   What I was after when I asked EPAM to post Misra related patches was to
   get a sense of the impact and a sense of how easily resolvable issues are.
   But I wouldn't expect a full resolution at this stage, if there
   is controversy. 

   So maybe we can handle these in a different way. From my PoV, it would be 
good 
   enough if key reviewers communicated per example whether
   - They accept that fixing the issue would be beneficial
   - What concerns they have
   - And how much they would fight for or against such a patch
 (using the -2 ... +2 scale as outlined in "EXPRESSING AGREEMENT AND 
 DISAGREEMENT" in https://xenproject.org/developers/governance/#decisions
   
   Clearly there can be some discussion, but we don't really need to "fight
   to the end" over these. 

3: Or we could change approach completely and go for a more high-level
   design and/or analysis based approach before we do anything else. I will 
expand 
   further down.

My personal preference would be to use 2 for a few patches, followed by 
3 as it gives us a different perspective.

Let me outline my thinking on 3:

There are a few things about Misra that we do not yet fully understand on a
number of different dimensions:
a) Issues are either mandatory or advisory. The scale changes depending on 
   the required level of safety (expressed in ASIL A-D).
b) There will likely be clusters of Misra rules we likely violate frequently
   and others we are hardly or not affected by   

We 

Re: [Xen-devel] [OSSTEST PATCH] jessie: Drop use of jessie-updates

[Juergen Groß]

On 2/27/19 12:46 PM, Ian Jackson wrote:

The Release file is out of date on our mirror, due to jessie's
retirement into LTS.

CC: Juergen Gross 
Signed-off-by: Ian Jackson 


Release-acked-by: Juergen Gross 


Juergen

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

Re: [Xen-devel] [PATCH 1/1] tools/ocaml: Dup2 /dev/null to stdin in daemonize()

[Juergen Groß]

On 2/27/19 11:46 AM, Andrew Cooper wrote:

On 27/02/2019 10:33, Christian Lindig wrote:

Don't close stdin in daemonize() but dup2 /dev/null instead. This avoids
fd 0 being reused and potentially written to.

Signed-off-by: Christian Lindig 


Possibly worth noting that this fixes a bug whereby /dev/xen/evtchn
reliably gets opened on fd 0.

I can fix the wording up on commit if there are no other concerns.

Reviewed-by: Andrew Cooper , and CC'ing
Juergen for 4.12


Release-acked-by: Juergen Gross 


Juergen

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

Re: [Xen-devel] [PATCH 0/3] mwait support for AMD processors

[Woods, Brian]

On 2/27/19 2:51 AM, Jan Beulich wrote:
 On 26.02.19 at 17:54,  wrote:
>> On 2/26/19 10:37 AM, Jan Beulich wrote:
>> On 26.02.19 at 17:25,  wrote:
 Correct me if I'm wrong, but the Xen's acpi-idle implementation is
 dependent on dom0 using a AML interpreter and then giving that data back
 to Xen.  I've heard that this doesn't always work correctly on PV dom0s
 and doesn't work at all on PVH dom0s.
>>>
>>> For C2 and deeper (using entering methods other than HLT) - yes.
>>> The use of HLT is the default with the assumption that this will put
>>> the system in C1 (i.e. with a pretty low wakeup latency); see
>>> default_idle(), cpuidle_init_cpu(), and acpi_idle_do_entry().
>>
>> Well, assuming C2 is enabled (which I was assume is the default case),
>> HLT roughly puts the processor in C2 rather than C1.  On my test system,
>> the debug console output for the cx tables only output HLT for C1 (which
>> is wrong).
>>
>> Rather than depending on dom0, which is shaky, and not having an AML
>> interpreter, it seems the best solution is to hardcode the values in
>> like Intel does.  If Xen had an AML interpreter, I'd agree doing things
>> differently (reading in the ACPI tables) would be best.  But given the
>> resources Xen has at the moment, this seems like the safest solution and
>> is better than using HLT (which is C2 assuming it's enabled) as the
>> default idle method like Xen is using now.
>>
>> It comes down to sometimes (when C2 is diabled in BIOS) using C1
>> thinking it's C2, or without the patches in the common case using C2
>> thinking it's C1.
> 
> So in one of our idle routines, how would one go about entering
> C1 or C2 depending on wakeup latency requirements? I'm having a
> hard time seeing how HLT can be used for both (without a reboot
> cycle and a BIOS option change in between). Yet if there's only
> one state that can be entered, then it's merely cosmetic whether
> it gets called C1 or C2 in the debug output.
> 
> Anyway - I guess we need to continue this discussion (if necessary)
> once I got around to actually look at the patches.
> 
> Jan
> 
> Does this answer the questions?

C2/CC6 enabled (default our internal MBs [and in general I'd assume])
HLT -> C2/CC6*
mwait -> C1/CC1

C2/CC6 disabled in BIOS
HLT -> C1/CC1
mwait -> C1/CC1

* HLT doesn't directly call C2/CC6 but it has a small timer then flushes 
the caches and puts it in C2/CC6.  Effectively it's the same but not 
exactly.

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

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

[osstest service owner]

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

Failures :-/ but no regressions.

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

version targeted for testing:
 xen  346e7d0f4b2179b9e0b09f4ebc98cbb3aae39a2c
baseline version:
 xen  e72ecc7615410e5bf1a1c9a4c7772322c16eeb82

Last test of basis   133382  2019-02-22 22:00:38 Z4 days
Failing since133430  2019-02-25 23:00:55 Z1 days8 attempts
Testing same since   133446  2019-02-26 20:00:42 Z0 days4 attempts


People who touched revisions under test:
  Andrew Cooper 
  Jan Beulich 
  Julien Grall 
  Norbert Manthey 
  Paul Durrant 
  Tim Deegan 

jobs:
 build-arm64-xsm  pass
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-arm64-arm64-xl-xsm  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 :

To xenbits.xen.org:/home/xen/git/xen.git
   e72ecc7615..346e7d0f4b  346e7d0f4b2179b9e0b09f4ebc98cbb3aae39a2c -> smoke

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

[Xen-devel] [PATCH L1TF v9 6/7] x86/hvm: add nospec to hvmop param

[Norbert Manthey]

The params array in hvm can be accessed with get and set functions.
As the index is guest controlled, make sure no out-of-bound accesses
can be performed.

As we cannot influence how future compilers might modify the
instructions that enforce the bounds, we furthermore block speculation,
so that the update is visible in the architectural state.

This is part of the speculative hardening effort.

Signed-off-by: Norbert Manthey 
Acked-by: Jan Beulich 

---

Notes:
  v9: fixed inline comments
  added acked-by

 xen/arch/x86/hvm/hvm.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4135,6 +4135,9 @@ static int hvmop_set_param(
 if ( a.index >= HVM_NR_PARAMS )
 return -EINVAL;
 
+/* Make sure the above bound check is not bypassed during speculation. */
+block_speculation();
+
 d = rcu_lock_domain_by_any_id(a.domid);
 if ( d == NULL )
 return -ESRCH;
@@ -4401,6 +4404,9 @@ static int hvmop_get_param(
 if ( a.index >= HVM_NR_PARAMS )
 return -EINVAL;
 
+/* Make sure the above bound check is not bypassed during speculation. */
+block_speculation();
+
 d = rcu_lock_domain_by_any_id(a.domid);
 if ( d == NULL )
 return -ESRCH;
-- 
2.7.4




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B



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

[Xen-devel] [PATCH L1TF v9 5/7] common/memory: block speculative out-of-bound accesses

[Norbert Manthey]

The get_page_from_gfn method returns a pointer to a page that belongs
to a gfn. Before returning the pointer, the gfn is checked for being
valid. Under speculation, these checks can be bypassed, so that
the function get_page is still executed partially. Consequently, the
function page_get_owner_and_reference might be executed partially as
well. In this function, the computed pointer is accessed, resulting in
a speculative out-of-bound address load. As the gfn can be controlled by
a guest, this access is problematic.

To mitigate the root cause, an lfence instruction is added via the
evaluate_nospec macro. To make the protection generic, we do not
introduce the lfence instruction for this single check, but add it to
the mfn_valid function. This way, other potentially problematic accesses
are protected as well.

This is part of the speculative hardening effort.

Signed-off-by: Norbert Manthey 
Acked-by: Jan Beulich 

---
 xen/common/pdx.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/xen/common/pdx.c b/xen/common/pdx.c
--- a/xen/common/pdx.c
+++ b/xen/common/pdx.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /* Parameters for PFN/MADDR compression. */
 unsigned long __read_mostly max_pdx;
@@ -33,8 +34,9 @@ unsigned long __read_mostly pdx_group_valid[BITS_TO_LONGS(
 
 bool __mfn_valid(unsigned long mfn)
 {
-return likely(mfn < max_page) &&
-   likely(!(mfn & pfn_hole_mask)) &&
+if ( unlikely(evaluate_nospec(mfn >= max_page)) )
+return false;
+return likely(!(mfn & pfn_hole_mask)) &&
likely(test_bit(pfn_to_pdx(mfn) / PDX_GROUP_COUNT,
pdx_group_valid));
 }
-- 
2.7.4




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B



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

[Xen-devel] [PATCH L1TF v9 7/7] common/grant_table: block speculative out-of-bound accesses

[Norbert Manthey]

Guests can issue grant table operations and provide guest controlled
data to them. This data is also used for memory loads. To avoid
speculative out-of-bound accesses, we use the array_index_nospec macro
where applicable. However, there are also memory accesses that cannot
be protected by a single array protection, or multiple accesses in a
row. To protect these, a nospec barrier is placed between the actual
range check and the access via the block_speculation macro.

As different versions of grant tables use structures of different size,
and the status is encoded in an array for version 2, speculative
execution might perform out-of-bound accesses of version 2 while
the table is actually using version 1. Hence, speculation is prevented
when accessing memory based on the grant table version.

Speculative execution is not blocked in case one of the following
properties is true:
 - path cannot be triggered by the guest
 - path does not return to the guest
 - path does not result in an out-of-bound access
 - path cannot be executed repeatedly
Only the combination of the above properties allows to actually leak
continuous chunks of memory. Therefore, we only add the penalty of
protective mechanisms in case a potential speculative out-of-bound
access matches all the above properties.

This is part of the speculative hardening effort.

Signed-off-by: Norbert Manthey 

---

Notes:
  v8: extended commit message with reason when to block speculation
  fix order assert_unreachable and block_speculation
  add block_speculation to gnttab_get_status_frame_mfn
  protect version comparison in gnttab_map_frame and release_grant_for_copy

 xen/common/grant_table.c | 96 +---
 1 file changed, 74 insertions(+), 22 deletions(-)

diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -37,6 +37,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -203,8 +204,9 @@ static inline unsigned int nr_status_frames(const struct 
grant_table *gt)
 }
 
 #define MAPTRACK_PER_PAGE (PAGE_SIZE / sizeof(struct grant_mapping))
-#define maptrack_entry(t, e) \
-((t)->maptrack[(e)/MAPTRACK_PER_PAGE][(e)%MAPTRACK_PER_PAGE])
+#define maptrack_entry(t, e)   
\
+((t)->maptrack[array_index_nospec(e, (t)->maptrack_limit) /
\
+MAPTRACK_PER_PAGE][(e) % 
MAPTRACK_PER_PAGE])
 
 static inline unsigned int
 nr_maptrack_frames(struct grant_table *t)
@@ -226,10 +228,23 @@ nr_maptrack_frames(struct grant_table *t)
 static grant_entry_header_t *
 shared_entry_header(struct grant_table *t, grant_ref_t ref)
 {
-if ( t->gt_version == 1 )
+switch ( t->gt_version )
+{
+case 1:
+/* Returned values should be independent of speculative execution */
+block_speculation();
 return (grant_entry_header_t*)_entry_v1(t, ref);
-else
+
+case 2:
+/* Returned values should be independent of speculative execution */
+block_speculation();
 return _entry_v2(t, ref).hdr;
+}
+
+ASSERT_UNREACHABLE();
+block_speculation();
+
+return NULL;
 }
 
 /* Active grant entry - used for shadowing GTF_permit_access grants. */
@@ -634,14 +649,24 @@ static unsigned int nr_grant_entries(struct grant_table 
*gt)
 case 1:
 BUILD_BUG_ON(f2e(INITIAL_NR_GRANT_FRAMES, 1) <
  GNTTAB_NR_RESERVED_ENTRIES);
+
+/* Make sure we return a value independently of speculative execution 
*/
+block_speculation();
 return f2e(nr_grant_frames(gt), 1);
+
 case 2:
 BUILD_BUG_ON(f2e(INITIAL_NR_GRANT_FRAMES, 2) <
  GNTTAB_NR_RESERVED_ENTRIES);
+
+/* Make sure we return a value independently of speculative execution 
*/
+block_speculation();
 return f2e(nr_grant_frames(gt), 2);
 #undef f2e
 }
 
+ASSERT_UNREACHABLE();
+block_speculation();
+
 return 0;
 }
 
@@ -963,9 +988,13 @@ map_grant_ref(
 PIN_FAIL(unlock_out, GNTST_bad_gntref, "Bad ref %#x for d%d\n",
  op->ref, rgt->domain->domain_id);
 
-act = active_entry_acquire(rgt, op->ref);
+/* This call ensures the above check cannot be bypassed speculatively */
 shah = shared_entry_header(rgt, op->ref);
-status = rgt->gt_version == 1 ? >flags : _entry(rgt, op->ref);
+act = active_entry_acquire(rgt, op->ref);
+
+/* Make sure we do not access memory speculatively */
+status = evaluate_nospec(rgt->gt_version == 1) ? >flags
+ : _entry(rgt, op->ref);
 
 /* If already pinned, check the active domid and avoid refcnt overflow. */
 if ( act->pin &&
@@ -987,7 +1016,7 @@ map_grant_ref(
 
 if ( !act->pin )
 {
-unsigned long gfn = rgt->gt_version == 1 ?
+unsigned long gfn = 

[Xen-devel] [PATCH L1TF v9 4/7] is_hvm/pv_domain: block speculation

[Norbert Manthey]

When checking for being an hvm domain, or PV domain, we have to make
sure that speculation cannot bypass that check, and eventually access
data that should not end up in cache for the current domain type.

This is part of the speculative hardening effort.

Signed-off-by: Norbert Manthey 
Acked-by: Jan Beulich 

---
 xen/include/xen/sched.h | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -922,7 +922,8 @@ void watchdog_domain_destroy(struct domain *d);
 
 static inline bool is_pv_domain(const struct domain *d)
 {
-return IS_ENABLED(CONFIG_PV) ? d->guest_type == guest_type_pv : false;
+return IS_ENABLED(CONFIG_PV)
+   ? evaluate_nospec(d->guest_type == guest_type_pv) : false;
 }
 
 static inline bool is_pv_vcpu(const struct vcpu *v)
@@ -953,7 +954,8 @@ static inline bool is_pv_64bit_vcpu(const struct vcpu *v)
 #endif
 static inline bool is_hvm_domain(const struct domain *d)
 {
-return IS_ENABLED(CONFIG_HVM) ? d->guest_type == guest_type_hvm : false;
+return IS_ENABLED(CONFIG_HVM)
+   ? evaluate_nospec(d->guest_type == guest_type_hvm) : false;
 }
 
 static inline bool is_hvm_vcpu(const struct vcpu *v)
-- 
2.7.4




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B



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

[Xen-devel] [PATCH L1TF v9 3/7] is_control_domain: block speculation

[Norbert Manthey]

Checks of domain properties, such as is_hardware_domain or is_hvm_domain,
might be bypassed by speculatively executing these instructions. A reason
for bypassing these checks is that these macros access the domain
structure via a pointer, and check a certain field. Since this memory
access is slow, the CPU assumes a returned value and continues the
execution.

In case an is_control_domain check is bypassed, for example during a
hypercall, data that should only be accessible by the control domain could
be loaded into the cache.

This is part of the speculative hardening effort.

Signed-off-by: Norbert Manthey 
Acked-by: Jan Beulich 

---
 xen/include/xen/sched.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -913,10 +913,10 @@ void watchdog_domain_destroy(struct domain *d);
  *(that is, this would not be suitable for a driver domain)
  *  - There is never a reason to deny the hardware domain access to this
  */
-#define is_hardware_domain(_d) ((_d) == hardware_domain)
+#define is_hardware_domain(_d) evaluate_nospec((_d) == hardware_domain)
 
 /* This check is for functionality specific to a control domain */
-#define is_control_domain(_d) ((_d)->is_privileged)
+#define is_control_domain(_d) evaluate_nospec((_d)->is_privileged)
 
 #define VM_ASSIST(d, t) (test_bit(VMASST_TYPE_ ## t, &(d)->vm_assist))
 
-- 
2.7.4




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B



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

[Xen-devel] [PATCH L1TF v9 2/7] nospec: introduce evaluate_nospec

[Norbert Manthey]

Since the L1TF vulnerability of Intel CPUs, loading hypervisor data into
L1 cache is problematic, because when hyperthreading is used as well, a
guest running on the sibling core can leak this potentially secret data.

To prevent these speculative accesses, we block speculation after
accessing the domain property field by adding lfence instructions. This
way, the CPU continues executing and loading data only once the condition
is actually evaluated.

As this protection is typically used in if statements, the lfence has to
come in a compatible way. Therefore, a function that returns true after an
lfence instruction is introduced. To protect both branches after a
conditional, an lfence instruction has to be added for the two branches.
To be able to block speculation after several evaluations, the generic
barrier macro block_speculation is also introduced.

As the L1TF vulnerability is only present on the x86 architecture, there is
no need to add protection for other architectures. Hence, the introduced
functions are defined but empty.

On the x86 architecture, by default, the lfence instruction is not present
either. Only when a L1TF vulnerable platform is detected, the lfence
instruction is patched in via alternative patching. Similarly, PV guests
are protected wrt L1TF by default, so that the protection is furthermore
disabled in case HVM is exclueded via the build configuration.

Introducing the lfence instructions catches a lot of potential leaks with
a simple unintrusive code change. During performance testing, we did not
notice performance effects.

This is part of the speculative hardening effort.

Signed-off-by: Norbert Manthey 
Acked-by: Julien Grall 
---

Notes:
  v9: fixed indentation (ARM)
  dropped CONFIG_HVM in evaluate_nospec
  dropped cast in block_speculation

 xen/include/asm-arm/nospec.h | 25 +
 xen/include/asm-x86/nospec.h | 39 +++
 xen/include/xen/nospec.h |  1 +
 3 files changed, 65 insertions(+)
 create mode 100644 xen/include/asm-arm/nospec.h
 create mode 100644 xen/include/asm-x86/nospec.h

diff --git a/xen/include/asm-arm/nospec.h b/xen/include/asm-arm/nospec.h
new file mode 100644
--- /dev/null
+++ b/xen/include/asm-arm/nospec.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright 2018 Amazon.com, Inc. or its affiliates. All Rights Reserved. */
+
+#ifndef _ASM_ARM_NOSPEC_H
+#define _ASM_ARM_NOSPEC_H
+
+static inline bool evaluate_nospec(bool condition)
+{
+return condition;
+}
+
+static inline void block_speculation(void)
+{
+}
+
+#endif /* _ASM_ARM_NOSPEC_H */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/asm-x86/nospec.h b/xen/include/asm-x86/nospec.h
new file mode 100644
--- /dev/null
+++ b/xen/include/asm-x86/nospec.h
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright 2018 Amazon.com, Inc. or its affiliates. All Rights Reserved. */
+
+#ifndef _ASM_X86_NOSPEC_H
+#define _ASM_X86_NOSPEC_H
+
+#include 
+
+/* Allow to insert a read memory barrier into conditionals */
+static always_inline bool barrier_nospec_true(void)
+{
+#ifdef CONFIG_HVM
+alternative("", "lfence", X86_FEATURE_SC_L1TF_VULN);
+#endif
+return true;
+}
+
+/* Allow to protect evaluation of conditionasl with respect to speculation */
+static always_inline bool evaluate_nospec(bool condition)
+{
+return condition ? barrier_nospec_true() : !barrier_nospec_true();
+}
+
+/* Allow to block speculative execution in generic code */
+static always_inline void block_speculation(void)
+{
+barrier_nospec_true();
+}
+
+#endif /* _ASM_X86_NOSPEC_H */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/xen/nospec.h b/xen/include/xen/nospec.h
--- a/xen/include/xen/nospec.h
+++ b/xen/include/xen/nospec.h
@@ -8,6 +8,7 @@
 #define XEN_NOSPEC_H
 
 #include 
+#include 
 
 /**
  * array_index_mask_nospec() - generate a ~0 mask when index < size, 0 
otherwise
-- 
2.7.4




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B



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

[Xen-devel] [PATCH L1TF v9 1/7] spec: add l1tf-barrier

[Norbert Manthey]

To control the runtime behavior on L1TF vulnerable platforms better, the
command line option l1tf-barrier is introduced. This option controls
whether on vulnerable x86 platforms the lfence instruction is used to
prevent speculative execution from bypassing the evaluation of
conditionals that are protected with the evaluate_nospec macro.

By now, Xen is capable of identifying L1TF vulnerable hardware. However,
this information cannot be used for alternative patching, as a CPU feature
is required. To control alternative patching with the command line option,
a new x86 feature "X86_FEATURE_SC_L1TF_VULN" is introduced. This feature
is used to patch the lfence instruction into the arch_barrier_nospec_true
function. The feature is enabled only if L1TF vulnerable hardware is
detected and the command line option does not prevent using this feature.

The status of hyperthreading is considered when automatically enabling
adding the lfence instruction. Since platforms without hyperthreading can
still be vulnerable to L1TF in case the L1 cache is not flushed properly,
the additional lfence instructions are patched in if either hyperthreading
is enabled, or L1 cache flushing is missing.

This is part of the speculative hardening effort.

Signed-off-by: Norbert Manthey 
Reviewed-by: Jan Beulich 

---
 docs/misc/xen-command-line.pandoc | 14 ++
 xen/arch/x86/spec_ctrl.c  | 17 +++--
 xen/include/asm-x86/cpufeatures.h |  1 +
 xen/include/asm-x86/spec_ctrl.h   |  1 +
 4 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/docs/misc/xen-command-line.pandoc 
b/docs/misc/xen-command-line.pandoc
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -483,9 +483,9 @@ accounting for hardware capabilities as enumerated via 
CPUID.
 
 Currently accepted:
 
-The Speculation Control hardware features `ibrsb`, `stibp`, `ibpb`,
-`l1d-flush` and `ssbd` are used by default if available and applicable.  They 
can
-be ignored, e.g. `no-ibrsb`, at which point Xen won't use them itself, and
+The Speculation Control hardware features `ibrsb`, `stibp`, `ibpb`, 
`l1d-flush`,
+`l1tf-barrier` and `ssbd` are used by default if available and applicable.  
They
+can be ignored, e.g. `no-ibrsb`, at which point Xen won't use them itself, and
 won't offer them to guests.
 
 ### cpuid_mask_cpu
@@ -1896,7 +1896,7 @@ By default SSBD will be mitigated at runtime (i.e 
`ssbd=runtime`).
 ### spec-ctrl (x86)
 > `= List of [ , xen=, {pv,hvm,msr-sc,rsb}=,
 >  bti-thunk=retpoline|lfence|jmp, {ibrs,ibpb,ssbd,eager-fpu,
->  l1d-flush}= ]`
+>  l1d-flush,l1tf-barrier}= ]`
 
 Controls for speculative execution sidechannel mitigations.  By default, Xen
 will pick the most appropriate mitigations based on compiled in support,
@@ -1962,6 +1962,12 @@ Irrespective of Xen's setting, the feature is 
virtualised for HVM guests to
 use.  By default, Xen will enable this mitigation on hardware believed to be
 vulnerable to L1TF.
 
+On hardware vulnerable to L1TF, the `l1tf-barrier=` option can be used to force
+or prevent Xen from protecting evaluations inside the hypervisor with a barrier
+instruction to not load potentially secret information into L1 cache.  By
+default, Xen will enable this mitigation on hardware believed to be vulnerable
+to L1TF.
+
 ### sync_console
 > `= `
 
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -21,6 +21,7 @@
 #include 
 #include 
 
+#include 
 #include 
 #include 
 #include 
@@ -50,6 +51,7 @@ bool __read_mostly opt_ibpb = true;
 bool __read_mostly opt_ssbd = false;
 int8_t __read_mostly opt_eager_fpu = -1;
 int8_t __read_mostly opt_l1d_flush = -1;
+int8_t __read_mostly opt_l1tf_barrier = -1;
 
 bool __initdata bsp_delay_spec_ctrl;
 uint8_t __read_mostly default_xen_spec_ctrl;
@@ -91,6 +93,8 @@ static int __init parse_spec_ctrl(const char *s)
 if ( opt_pv_l1tf_domu < 0 )
 opt_pv_l1tf_domu = 0;
 
+opt_l1tf_barrier = 0;
+
 disable_common:
 opt_rsb_pv = false;
 opt_rsb_hvm = false;
@@ -157,6 +161,8 @@ static int __init parse_spec_ctrl(const char *s)
 opt_eager_fpu = val;
 else if ( (val = parse_boolean("l1d-flush", s, ss)) >= 0 )
 opt_l1d_flush = val;
+else if ( (val = parse_boolean("l1tf-barrier", s, ss)) >= 0 )
+opt_l1tf_barrier = val;
 else
 rc = -EINVAL;
 
@@ -248,7 +254,7 @@ static void __init print_details(enum ind_thunk thunk, 
uint64_t caps)
"\n");
 
 /* Settings for Xen's protection, irrespective of guests. */
-printk("  Xen settings: BTI-Thunk %s, SPEC_CTRL: %s%s, Other:%s%s\n",
+printk("  Xen settings: BTI-Thunk %s, SPEC_CTRL: %s%s, Other:%s%s%s\n",
thunk == THUNK_NONE  ? "N/A" :
thunk == THUNK_RETPOLINE ? "RETPOLINE" :
thunk == THUNK_LFENCE

[Xen-devel] L1TF Patch Series v8

[Norbert Manthey]

This patch series attempts to mitigate the issue that have been raised in the
XSA-289 (https://xenbits.xen.org/xsa/advisory-289.html). To block speculative
execution on Intel hardware, an lfence instruction is required to make sure
that selected checks are not bypassed. Speculative out-of-bound accesses can
be prevented by using the array_index_nospec macro.

The major change compared to version 8 is in grant_table handling, protecting
a few more version comparisons.

Best,
Norbert



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B



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

Re: [Xen-devel] [PATCH v4.1 4/6] xen/x86: Allow stubdom access to irq created for msi.

[Marek Marczykowski]

On Wed, Feb 27, 2019 at 04:07:54AM -0700, Jan Beulich wrote:
> >>> On 08.02.19 at 11:17,  wrote:
> > There is one code path where I haven't managed to properly extract
> > possible stubdomain in use:
> > pci_remove_device()
> >  -> pci_cleanup_msi()
> >-> msi_free_irqs()
> >  -> msi_free_irq()
> >-> destroy_irq()
> > 
> > For now I've hardcoded hardware_domain there (in msi_free_irqs). Can it 
> > happen
> > when device is still assigned to some domU?
> 
> In case this question is still open: No, it can't with current code,
> and provided Dom0 behaves correctly.

Thanks for confirmation.

> > @@ -368,13 +369,13 @@ static int __init hpet_assign_irq(struct 
> > hpet_event_channel *ch)
> >  {
> >  int irq;
> >  
> > -if ( (irq = create_irq(NUMA_NO_NODE)) < 0 )
> > +if ( (irq = create_irq(NUMA_NO_NODE, hardware_domain)) < 0 )
> >  return irq;
> >  
> >  ch->msi.irq = irq;
> >  if ( hpet_setup_msi_irq(ch) )
> >  {
> > -destroy_irq(irq);
> > +destroy_irq(irq, hardware_domain);
> >  return -EINVAL;
> >  }
> 
> Why don't you take the opportunity here (and elsewhere) and properly
> remove hwdom access to such internal-to-Xen IRQs? Simply pass NULL
> here, and skip permission granting in this case (create_irq() already
> checks for NULL anyway).

Already queued for v5, per Roger's review.

> > @@ -190,19 +190,19 @@ int create_irq(nodeid_t node)
> >  desc->arch.used = IRQ_UNUSED;
> >  irq = ret;
> >  }
> > -else if ( hardware_domain )
> > +else if ( dm_domain )
> >  {
> > -ret = irq_permit_access(hardware_domain, irq);
> > +ret = irq_permit_access(dm_domain, irq);
> 
> Doesn't this imply that Dom0 has no way of cleaning up after the
> guest/stubdom pair? IOW I wonder whether both dm and hwdom
> should be granted access.

See discussion with Roger on this very patch.
In short: since permissions are stored in domain struct, not irq, there
is not much to cleanup after domain destruction. Also, toolstack in dom0
has no idea about IRQs allocated by stubdomain, so it couldn't do such
cleanup anyway.

> > @@ -2095,7 +2099,9 @@ int map_domain_pirq(
> >  irq = info->arch.irq;
> >  }
> >  msi_desc->irq = -1;
> > -msi_free_irq(msi_desc);
> > +msi_free_irq(msi_desc,
> > + current->domain->target == d ? current->domain
> > +  : hardware_domain);
> 
> Note how ->irq gets set to -1 prior to the call (and also in at least
> one other instance), which will lead to skipping of the destroy_irq()
> call, and hence skipping of the permission removal. Or wait, that's
> going to be taken care of in the caller as it seems. If this is also
> your understanding, then please add a sentence to the description
> pointing this out. The split logic isn't really helpful here (I know it
> was me who wrote it, in an attempt to avoid re-writing everything
> basically from scratch).

Yes, that matches my understanding - the caller will call on error
destroy_irq(), if it called create_irq() before (which may not always be
the case - and I think this is why it isn't destroyed here).

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


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

Re: [Xen-devel] [PATCH v4 5/6] xen/x86: add PHYSDEVOP_msi_set_enable

[Marek Marczykowski]

On Wed, Feb 27, 2019 at 04:41:37AM -0700, Jan Beulich wrote:
> >>> On 07.02.19 at 01:07,  wrote:
> > --- a/xen/arch/x86/msi.c
> > +++ b/xen/arch/x86/msi.c
> > @@ -1474,6 +1474,30 @@ int pci_restore_msi_state(struct pci_dev *pdev)
> >  return 0;
> >  }
> >  
> > +int msi_msix_set_enable(struct pci_dev *pdev, int mode, int enable)
> 
> unsigned int mode, bool enable
> 
> I'm also not happy about the function name. Perhaps simply msi_enable()
> or msi_control()?

Ok, will change to msi_control().

> > +{
> > +int ret;
> > +
> > +ret = xsm_msi_set_enable(XSM_DM_PRIV, pdev->domain,
> > + (pdev->seg << 16) | (pdev->bus << 8) | 
> > pdev->devfn,
> > + mode, enable);
> > +if ( ret )
> > +return ret;
> > +
> > +switch ( mode )
> > +{
> > +case PHYSDEVOP_MSI_SET_ENABLE_MSI:
> > +msi_set_enable(pdev, enable);
> > +break;
> > +
> > +case PHYSDEVOP_MSI_SET_ENABLE_MSIX:
> > +msix_set_enable(pdev, enable);
> > +break;
> > +}
> 
> What about a call to pci_intx()? 

Should pci_intx(dev, !enable) be called in all those cases?

> And what about invocations for
> the wrong device (e.g. MSI-X request for MSI-X incapable device)?

Looking at msi(x)_set_enable(), it is no-op for incapable devices, but
if the function would do anything else, indeed such check should be
added. Is pci_find_cap_offset(..., PCI_CAP_ID_MSI(X)) the correct way
of doing that?

> > --- a/xen/arch/x86/physdev.c
> > +++ b/xen/arch/x86/physdev.c
> > @@ -671,6 +671,30 @@ ret_t do_physdev_op(int cmd, 
> > XEN_GUEST_HANDLE_PARAM(void) arg)
> >  break;
> >  }
> >  
> > +case PHYSDEVOP_msi_set_enable: {
> > +struct physdev_msi_set_enable op;
> > +struct pci_dev *pdev;
> > +
> > +ret = -EFAULT;
> > +if ( copy_from_guest(, arg, 1) )
> > +break;
> > +
> > +ret = -EINVAL;
> > +if ( op.mode != PHYSDEVOP_MSI_SET_ENABLE_MSI &&
> > + op.mode != PHYSDEVOP_MSI_SET_ENABLE_MSIX )
> > +break;
> > +
> > +pcidevs_lock();
> > +pdev = pci_get_pdev(op.seg, op.bus, op.devfn);
> > +if ( pdev )
> > +ret = msi_msix_set_enable(pdev, op.mode, !!op.enable);
> 
> Unnecessary !! .
> 
> > +else
> > +ret = -ENODEV;
> > +pcidevs_unlock();
> > +break;
> > +
> > +}
> 
> Stray blank line above here.
> 
> > --- a/xen/include/public/physdev.h
> > +++ b/xen/include/public/physdev.h
> > @@ -344,6 +344,21 @@ struct physdev_dbgp_op {
> >  typedef struct physdev_dbgp_op physdev_dbgp_op_t;
> >  DEFINE_XEN_GUEST_HANDLE(physdev_dbgp_op_t);
> >  
> > +#define PHYSDEVOP_MSI_SET_ENABLE_MSI  0
> > +#define PHYSDEVOP_MSI_SET_ENABLE_MSIX 1
> > +
> > +#define PHYSDEVOP_msi_set_enable   32
> > +struct physdev_msi_set_enable {
> 
> Can this please also be something like msi_control?

Sure.

> > +/* IN */
> > +uint16_t seg;
> > +uint8_t bus;
> > +uint8_t devfn;
> > +uint8_t mode;
> > +uint8_t enable;
> 
> "mode" and "enable" don't really make clear which of the two is the
> boolean, and which is the operation. I'd anyway prefer a single
> flags field with descriptive #define-s, which will also make more
> obvious how to extend this if need be.

You mean:

#define PHYSDEVOP_MSI_CONTROL_ENABLE 1
#define PHYSDEVOP_MSI_CONTROL_MSI2 
#define PHYSDEVOP_MSI_CONTROL_MSIX   4

Then use PHYSDEVOP_MSI_CONTROL_MSI(X) with or without
PHYSDEVOP_MSI_CONTROL_ENABLE ?

> > --- a/xen/include/xlat.lst
> > +++ b/xen/include/xlat.lst
> > @@ -106,6 +106,7 @@
> >  ?  physdev_restore_msi physdev.h
> >  ?  physdev_set_ioplphysdev.h
> >  ?  physdev_setup_gsi   physdev.h
> > +?  physdev_msi_set_enable  physdev.h
> 
> Please insert at the alphabetically correct place.
> 
> Jan
> 
> 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


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

Re: [Xen-devel] [PATCH v6 5/5] npt/shadow: allow getting foreign page table entries

[Wei Liu]

On Wed, Feb 27, 2019 at 12:09:05PM +0100, Roger Pau Monne wrote:
> Current npt and shadow code to get an entry will always return
> INVALID_MFN for foreign entries. Allow to return the entry mfn for
> foreign entries, like it's done for grant table entries.
> 
> Signed-off-by: Roger Pau Monné 
> Reviewed-by: Jan Beulich 

Reviewed-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v6 4/5] x86/mm: handle foreign mappings in p2m_entry_modify

[Wei Liu]

On Wed, Feb 27, 2019 at 12:09:04PM +0100, Roger Pau Monne wrote:
> So that the specific handling can be removed from
> atomic_write_ept_entry and be shared with npt and shadow code.
> 
> This commit also removes the check that prevent non-ept PVH dom0 from
> mapping foreign pages.
> 
> Signed-off-by: Roger Pau Monné 
> Reviewed-by: Kevin Tian 

Reviewed-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v6.1 3/5] p2m: change write_p2m_entry to return an error code

[Wei Liu]

On Wed, Feb 27, 2019 at 12:30:31PM +0100, Roger Pau Monne wrote:
> This is in preparation for also changing p2m_entry_modify to return an
> error code.
> 
> No functional change intended.
> 
> Signed-off-by: Roger Pau Monné 

Reviewed-by: Wei Liu 

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

Re: [Xen-devel] [PATCH 1/3] mwait-idle: add support for using halt

[Wei Liu]

On Mon, Feb 25, 2019 at 08:23:58PM +, Woods, Brian wrote:
> Some AMD processors can use a mixture of mwait and halt for accessing
> various c-states.  In preparation for adding support for AMD processors,
> update the mwait-idle driver to optionally use halt.
> 
> Signed-off-by: Brian Woods 
> ---
>  xen/arch/x86/cpu/mwait-idle.c | 40 +---
>  1 file changed, 33 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/arch/x86/cpu/mwait-idle.c b/xen/arch/x86/cpu/mwait-idle.c
> index f89c52f256..a063e39d60 100644
> --- a/xen/arch/x86/cpu/mwait-idle.c
> +++ b/xen/arch/x86/cpu/mwait-idle.c
> @@ -103,6 +103,11 @@ static const struct cpuidle_state {
>  
>  #define CPUIDLE_FLAG_DISABLED0x1
>  /*
> + * On certain AMD families that support mwait, only c1 can be reached by
> + * mwait and to reach c2, halt has to be used.
> + */
> +#define CPUIDLE_FLAG_USE_HALT0x2
> +/*
>   * Set this flag for states where the HW flushes the TLB for us
>   * and so we don't need cross-calls to keep it consistent.
>   * If this flag is set, SW flushes the TLB, so even if the
> @@ -783,8 +788,23 @@ static void mwait_idle(void)
>  
>   update_last_cx_stat(power, cx, before);
>  
> - if (cpu_is_haltable(cpu))
> - mwait_idle_with_hints(eax, MWAIT_ECX_INTERRUPT_BREAK);
> + if (cpu_is_haltable(cpu)) {
> + struct cpu_info *info;
> + switch (cx->entry_method) {
> + case ACPI_CSTATE_EM_FFH:
> + mwait_idle_with_hints(eax, MWAIT_ECX_INTERRUPT_BREAK);
> + break;
> + case ACPI_CSTATE_EM_HALT:

> + info = get_cpu_info();
> + spec_ctrl_enter_idle(info);
> + safe_halt();
> + spec_ctrl_exit_idle(info);

May I suggest you make this snippet a function? The same code snippet
appears a few lines above.

Wei.

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

[Xen-devel] [freebsd-master test] 133455: all pass - PUSHED

[osstest service owner]

flight 133455 freebsd-master real [real]
http://logs.test-lab.xenproject.org/osstest/logs/133455/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 freebsd  001b002f2baadcb1f78e1e2c74716f976ed6b6ed
baseline version:
 freebsd  559f0dfc7a5f8f6a3ba157087820ce5e93c21486

Last test of basis   133365  2019-02-22 09:19:08 Z5 days
Failing since133421  2019-02-25 09:19:25 Z2 days2 attempts
Testing same since   133455  2019-02-27 09:19:02 Z0 days1 attempts


People who touched revisions under test:
  0mp <0...@freebsd.org>
  andrew 
  asomers 
  bapt 
  bde 
  brueffer 
  bwidawsk 
  bz 
  dab 
  des 
  dim 
  emaste 
  glebius 
  hselasky 
  ian 
  imp 
  jah 
  jhibbits 
  jilles 
  jkim 
  kevans 
  kib 
  kp 
  luporl 
  manu 
  markj 
  mav 
  mckusick 
  mmacy 
  np 
  sef 
  sjg 
  sobomax 
  ume 
  vmaffione 
  wulf 

jobs:
 build-amd64-freebsd-againpass
 build-amd64-freebsd  pass
 build-amd64-xen-freebsd  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 :

To xenbits.xen.org:/home/xen/git/freebsd.git
   559f0dfc7a5..001b002f2ba  001b002f2baadcb1f78e1e2c74716f976ed6b6ed -> 
tested/master

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

[Xen-devel] [xen-unstable-smoke test] 133452: trouble: blocked/broken

[osstest service owner]

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

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64  broken
 build-arm64-xsm  broken
 build-armhf  broken
 build-amd64   4 host-install(4)broken REGR. vs. 133382
 build-arm64-xsm   4 host-install(4)broken REGR. vs. 133382
 build-armhf   4 host-install(4)broken REGR. vs. 133382

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

version targeted for testing:
 xen  346e7d0f4b2179b9e0b09f4ebc98cbb3aae39a2c
baseline version:
 xen  e72ecc7615410e5bf1a1c9a4c7772322c16eeb82

Last test of basis   133382  2019-02-22 22:00:38 Z4 days
Failing since133430  2019-02-25 23:00:55 Z1 days7 attempts
Testing same since   133446  2019-02-26 20:00:42 Z0 days3 attempts


People who touched revisions under test:
  Andrew Cooper 
  Jan Beulich 
  Julien Grall 
  Norbert Manthey 
  Paul Durrant 
  Tim Deegan 

jobs:
 build-arm64-xsm  broken  
 build-amd64  broken  
 build-armhf  broken  
 build-amd64-libvirt  blocked 
 test-armhf-armhf-xl  blocked 
 test-arm64-arm64-xl-xsm  blocked 
 test-amd64-amd64-xl-qemuu-debianhvm-i386 blocked 
 test-amd64-amd64-libvirt 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 broken
broken-job build-arm64-xsm broken
broken-job build-armhf broken
broken-step build-amd64 host-install(4)
broken-step build-arm64-xsm host-install(4)
broken-step build-armhf host-install(4)

Not pushing.


commit 346e7d0f4b2179b9e0b09f4ebc98cbb3aae39a2c
Author: Norbert Manthey 
Date:   Tue Feb 26 16:57:56 2019 +0100

x86/vioapic: block speculative out-of-bound accesses

When interacting with io apic, a guest can specify values that are used
as index to structures, and whose values are not compared against
upper bounds to prevent speculative out-of-bound accesses. This change
prevents these speculative accesses.

Furthermore, variables are initialized and the compiler is asked to not
optimized these initializations, as the uninitialized variables might be
used in a speculative out-of-bound access. Out of the four initialized
variables, two are potentially problematic, namely ones in the functions
vioapic_irq_positive_edge and vioapic_get_trigger_mode.

As the two problematic variables are both used in the common function
gsi_vioapic, the mitigation is implemented there. As the access pattern
of the currently non-guest-controlled functions might change in the
future as well, the other variables are initialized as well.

This is part of the speculative hardening effort.

Signed-off-by: Norbert Manthey 
Reviewed-by: Jan Beulich 
Release-acked-by: Juergen Gross 

commit 443d3ab6daee9bf77ec1cb2ea7e252fb0ce616a8
Author: Norbert Manthey 
Date:   Tue Feb 26 16:57:18 2019 +0100

evtchn: block speculative out-of-bound accesses

Guests can issue event channel interaction with guest specified data.
To avoid speculative out-of-bound accesses, we use the nospec macros,
or the domain_vcpu function. Where appropriate, we use the vcpu_id of
the seleceted vcpu instead of the parameter that can be influenced by
the guest, so that only one access needs to be protected.

This is part of the speculative hardening effort.

Signed-off-by: Norbert Manthey 

Re: [Xen-devel] [PATCH L1TF v8 9/9] common/grant_table: block speculative out-of-bound accesses

[Norbert Manthey]

On 2/25/19 17:46, Jan Beulich wrote:
 On 25.02.19 at 14:34,  wrote:
>> @@ -634,14 +649,24 @@ static unsigned int nr_grant_entries(struct 
>> grant_table *gt)
>>  case 1:
>>  BUILD_BUG_ON(f2e(INITIAL_NR_GRANT_FRAMES, 1) <
>>   GNTTAB_NR_RESERVED_ENTRIES);
>> +
>> +/* Make sure we return a value independently of speculative 
>> execution */
>> +block_speculation();
>>  return f2e(nr_grant_frames(gt), 1);
>> +
>>  case 2:
>>  BUILD_BUG_ON(f2e(INITIAL_NR_GRANT_FRAMES, 2) <
>>   GNTTAB_NR_RESERVED_ENTRIES);
>> +
>> +/* Make sure we return a value independently of speculative 
>> execution */
>> +block_speculation();
>>  return f2e(nr_grant_frames(gt), 2);
>>  #undef f2e
>>  }
>>  
>> +block_speculation();
>> +ASSERT_UNREACHABLE();
>> +
>>  return 0;
>>  }
> Personally I think the assertion should be first (also in
> shared_entry_header()), but that's nothing very important to
> change.
I will change the order.
>
> Below here, but before the next patch hunk, is _set_status(). If
> you think there's no need to change its gt_version check, then I
> think the commit message should (briefly) explain this.
>
>> @@ -1418,7 +1450,7 @@ unmap_common_complete(struct gnttab_unmap_common *op)
>>  struct page_info *pg;
>>  uint16_t *status;
>>  
>> -if ( !op->done )
>> +if ( evaluate_nospec(!op->done) )
>>  {
>>  /* unmap_common() didn't do anything - nothing to complete. */
>>  return;
> Just like above, below here (in the same function) is another version
> check you don't adjust, and there are further ones in gnttab_grow_table(),
> gnttab_setup_table(), and release_grant_for_copy().
>
>> @@ -2408,9 +2445,11 @@ acquire_grant_for_copy(
>>  PIN_FAIL(gt_unlock_out, GNTST_bad_gntref,
>>   "Bad grant reference %#x\n", gref);
>>  
>> -act = active_entry_acquire(rgt, gref);
>> +/* This call ensures the above check cannot be bypassed speculatively */
>>  shah = shared_entry_header(rgt, gref);
>> -if ( rgt->gt_version == 1 )
>> +act = active_entry_acquire(rgt, gref);
>> +
>> +if ( evaluate_nospec(rgt->gt_version == 1) )
>>  {
>>  sha2 = NULL;
>>  status = >flags;
> There's again a second version check further down in this function.
>
>> @@ -2945,7 +2987,7 @@ 
>> gnttab_set_version(XEN_GUEST_HANDLE_PARAM(gnttab_set_version_t) uop)
>>  struct grant_table *gt = currd->grant_table;
>>  grant_entry_v1_t reserved_entries[GNTTAB_NR_RESERVED_ENTRIES];
>>  int res;
>> -unsigned int i;
>> +unsigned int i, nr_ents;
>>  
>>  if ( copy_from_guest(, uop, 1) )
>>  return -EFAULT;
>> @@ -2969,7 +3011,8 @@ 
>> gnttab_set_version(XEN_GUEST_HANDLE_PARAM(gnttab_set_version_t) uop)
>>   * are allowed to be in use (xenstore/xenconsole keeps them mapped).
>>   * (You need to change the version number for e.g. kexec.)
>>   */
>> -for ( i = GNTTAB_NR_RESERVED_ENTRIES; i < nr_grant_entries(gt); i++ )
>> +nr_ents = nr_grant_entries(gt);
>> +for ( i = GNTTAB_NR_RESERVED_ENTRIES; i < nr_ents; i++ )
>>  {
>>  if ( read_atomic(&_active_entry(gt, i).pin) != 0 )
>>  {
> What about the various version accesses in this function? And
> while I think the one in gnttab_release_mappings() doesn't need
> adjustment, it should (also) be mentioned in the description. The
> one in gnttab_map_frame(9, otoh, looks as if it again needed
> adjustment.
>
> I would really like to ask that I (or someone else) don't need to
> go through and list remaining version checks again - after all I
> had done so for v6 already, and I didn't go through all of them
> again for v7 assuming that you would have worked through the
> entire set.

So, here is the annotation for all of them. Anyone that I did not
include in the list has been fixed in previous versions, or will be
fixed in the next version:

git grep -np version common/grant_table.c

common/grant_table.c:831:static int _set_status(unsigned gt_version,
common/grant_table.c:840:    if ( gt_version == 1 )

-> I do not see how out-of-bound accesses happen in the called functions
there.

common/grant_table.c=1444=unmap_common_complete(struct
gnttab_unmap_common *op)
common/grant_table.c:1469:    if ( rgt->gt_version == 1 )

-> I do not see how to be exploitable, as the shared_entry_header call
above just used an lfence.

common/grant_table.c=1761=gnttab_grow_table(struct domain *d, unsigned
int req_nr_frames)
common/grant_table.c:1800:    /* Status pages - version 2 */
common/grant_table.c:1801:    if ( gt->gt_version > 1 )

-> I do not see how out-of-bound access could happen. This calls
gnttab_populate_status_frames that allocates pages and should not touch
more memory than before

common/grant_table.c=1904=gnttab_setup_table(
common/grant_table.c:1955:  ((gt->gt_version > 1) &&

-> I do not see how an out-of-bound access 

Re: [Xen-devel] [PATCH for-4.12 RFC] xen/console: Handle NUL character in buffer sent via CONSOLEIO_write

[Wei Liu]

On Wed, Feb 27, 2019 at 03:25:22AM -0700, Jan Beulich wrote:
> >>> On 27.02.19 at 00:03,  wrote:
> > After upgrading Debian to Buster, I started noticing console mangling
> > when using zsh. This is happenning because output sent by zsh to the
> > console may contain NUL character in the middle of the buffer.
> > 
> > Linux is sending the buffer as it is to Xen console via CONSOLEIO_write.
> > However, the implementation in Xen considers NUL character is used to
> > terminate the buffer and therefore will ignore anything after it.
> > 
> > The actual documentation of CONSOLEIO_write is pretty limited. From the
> > declaration, the hypercall takes a buffer and size. So this could lead
> > to think the NUL character is allowed in the middle of the buffer.
> > 
> > This patch updates the console API to pass the size along the buffer
> > down so we can remove the reliance on buffer terminating by a NUL
> > character.
> 
> We don't need the behavior for internal producers, so I think the change
> touches way too much code. I think all you need to do is make the
> hypercall handler sense null characters, and perhaps simply invoke lower
> level handlers multiple times. Or replace them by something else (e.g. a
> blank).

I don't think replacing NULs with blank is the right thing to do. It's
not only about how human perceives the output, but also about scripting.

Wei.

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

Re: [Xen-devel] [PATCH for-4.12 RFC] xen/console: Handle NUL character in buffer sent via CONSOLEIO_write

[Wei Liu]

On Tue, Feb 26, 2019 at 11:03:51PM +, Julien Grall wrote:
> After upgrading Debian to Buster, I started noticing console mangling
> when using zsh. This is happenning because output sent by zsh to the
> console may contain NUL character in the middle of the buffer.
> 
> Linux is sending the buffer as it is to Xen console via CONSOLEIO_write.
> However, the implementation in Xen considers NUL character is used to
> terminate the buffer and therefore will ignore anything after it.
> 
> The actual documentation of CONSOLEIO_write is pretty limited. From the
> declaration, the hypercall takes a buffer and size. So this could lead
> to think the NUL character is allowed in the middle of the buffer.
> 
> This patch updates the console API to pass the size along the buffer
> down so we can remove the reliance on buffer terminating by a NUL
> character.
> 
> Signed-off-by: Julien Grall 
> 
> ---
> 
[...]
> @@ -527,7 +527,7 @@ static inline void xen_console_write_debug_port(const 
> char *buf, size_t len)
>  static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer, int 
> count)
>  {
>  char kbuf[128];
> -int kcount = 0;
> +unsigned int kcount = 0;
>  struct domain *cd = current->domain;
>  
>  while ( count > 0 )
> @@ -547,8 +547,8 @@ static long 
> guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer, int count)
>  /* Use direct console output as it could be interactive */
>  spin_lock_irq(_lock);
>  
> -sercon_puts(kbuf);
> -video_puts(kbuf);
> +sercon_puts(kbuf, kcount);
> +video_puts(kbuf, kcount);
>  

I think you missed the non-hwdom branch in the same function. It still
strips non-printable characters.

>  int console_suspend(void)
[...]
> diff --git a/xen/drivers/char/consoled.c b/xen/drivers/char/consoled.c
> index 552abf5766..3e849a2557 100644
> --- a/xen/drivers/char/consoled.c
> +++ b/xen/drivers/char/consoled.c
> @@ -77,7 +77,7 @@ size_t consoled_guest_rx(void)
>  
>  if ( idx >= BUF_SZ )
>  {
> -pv_console_puts(buf);
> +pv_console_puts(buf, BUF_SZ);
>  idx = 0;
>  }
>  }
> @@ -85,7 +85,7 @@ size_t consoled_guest_rx(void)
>  if ( idx )
>  {
>  buf[idx] = '\0';

Can this be deleted? Now that you've explicitly sized the buffer.

Wei.

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

[Xen-devel] [xen-4.10-testing test] 133419: regressions - trouble: blocked/broken/fail/pass

[osstest service owner]

flight 133419 xen-4.10-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/133419/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qemut-win10-i386 broken
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm  broken
 test-amd64-amd64-libvirt broken
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadowbroken
 test-amd64-amd64-amd64-pvgrub broken
 build-i386-pvops broken
 build-amd64-prev broken
 test-amd64-amd64-xl-pvhv2-amd broken
 build-armhf-pvopsbroken
 build-armhf  broken
 test-amd64-amd64-xl-qemut-win7-amd64 broken
 test-amd64-amd64-xl  broken
 build-amd64-prev  4 host-install(4)broken REGR. vs. 132966
 build-i386-pvops  4 host-install(4)broken REGR. vs. 132966
 build-armhf-pvops 4 host-install(4)broken REGR. vs. 132966
 build-armhf   4 host-install(4)broken REGR. vs. 132966
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm  broken in 
133292
 test-amd64-i386-xl-qemut-debianhvm-amd64  broken in 133292
 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm  broken in 133292
 test-amd64-i386-livepatchbroken  in 133292
 test-amd64-amd64-qemuu-nested-intel   broken in 133311
 test-amd64-i386-xl-qemut-win7-amd64   broken in 133311
 test-amd64-amd64-xl-credit1  broken  in 133311
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm  broken in 133311
 build-amd64-xsm  broken  in 18
 build-i386   broken  in 18
 build-i386-prev  broken  in 18
 build-amd64-pvopsbroken  in 18
 test-armhf-armhf-xl-credit1  broken  in 18
 test-armhf-armhf-xl  broken  in 18
 test-armhf-armhf-libvirt-raw broken  in 18
 build-i386-xsm   broken  in 18
 build-amd64  broken  in 18
 build-i386  2 hosts-allocate broken in 18 REGR. vs. 132966
 build-i386-prev 2 hosts-allocate broken in 18 REGR. vs. 132966
 build-amd64-xsm 2 hosts-allocate broken in 18 REGR. vs. 132966
 build-i386-xsm  2 hosts-allocate broken in 18 REGR. vs. 132966
 build-amd64 2 hosts-allocate broken in 18 REGR. vs. 132966
 build-amd64-pvops  4 host-install(4) broken in 18 REGR. vs. 132966
 build-armhf-libvirt  3 syslog-server broken in 18 REGR. vs. 132966
 build-i386-libvirt   broken  in 133359
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm broken in 133359
 test-amd64-i386-qemut-rhel6hvm-intel  broken in 133359
 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm broken in 133359
 test-amd64-amd64-rumprun-amd64broken in 133359
 build-i386-libvirt 4 host-install(4) broken in 133359 REGR. vs. 132966
 test-amd64-amd64-xl-rtds broken  in 133393
 test-amd64-i386-xl-qemut-ws16-amd64   broken in 133393
 test-amd64-i386-xl-shadowbroken  in 133393
 test-armhf-armhf-xl-cubietruckbroken in 133393
 test-armhf-armhf-xl-multivcpu broken in 133393
 test-amd64-i386-migrupgrade  broken  in 133393
 test-armhf-armhf-xl-arndale  broken  in 133393
 test-amd64-amd64-xl-multivcpu broken in 133393
 test-amd64-amd64-xl-qemuu-win10-i386  broken in 133393
 test-amd64-i386-xl-qemut-win10-i386   broken in 133393
 build-i386-prev   6 xen-buildfail REGR. vs. 132966
 test-amd64-amd64-xl-xsm   1 build-check(1)   running in 18
 build-amd64-rumprun   1 build-check(1)   running in 18
 test-xtf-amd64-amd64-11 build-check(1)   running in 18
 test-xtf-amd64-amd64-41 build-check(1)   running in 18
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 1 build-check(1) running in 
18
 test-amd64-amd64-xl-qemuu-ws16-amd64  1 build-check(1)   running in 18
 test-amd64-amd64-libvirt-vhd  1 build-check(1)   running in 18
 test-amd64-amd64-i386-pvgrub  1 build-check(1)   running in 18
 test-amd64-amd64-livepatch1 build-check(1)   running in 18
 test-xtf-amd64-amd64-31 build-check(1)   running in 18
 test-amd64-amd64-libvirt-pair  1 

Re: [Xen-devel] [PATCH v2 3/3] tools/cpu-policy: Add unit tests

[Wei Liu]

On Mon, Feb 25, 2019 at 03:47:15PM +, Andrew Cooper wrote:
> These will be extended with further libx86 work.
> 
> Fix the sorting of the CPUID_GUEST_NR_* constants, noticed while writing the
> unit tests.
> 
> Signed-off-by: Andrew Cooper 

Looks good to me. But I will let you and Jan figure out whether we
should use centralised gitignore or not.

Wei.

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

Re: [Xen-devel] [PATCH v2 2/3] libx86: Introduce a helper to deserialise cpuid_policy objects

[Wei Liu]

On Mon, Feb 25, 2019 at 03:47:14PM +, Andrew Cooper wrote:
> Signed-off-by: Andrew Cooper 
> Signed-off-by: Sergey Dyasli 
> Signed-off-by: Roger Pau Monné 

Reviewed-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v2 1/3] libx86: introduce a helper to deserialise msr_policy objects

[Wei Liu]

On Mon, Feb 25, 2019 at 03:47:13PM +, Andrew Cooper wrote:
> +
> +switch ( data.idx )
> +{
> +/*
> + * Assign data.val to 'field', checking for truncation if the
> + * backing storage for 'field' is smaller than uint64_t
> + */
> +#define ASSIGN(field)   \
> +({  \
> +if ( (typeof(field))data.val != data.val )  \
> +{   \
> +rc = -EOVERFLOW;\
> +goto err;   \
> +}   \
> +field = data.val;   \

Missing parentheses around "field" in the macro. Although I don't think
it will break in practice, it is better to follow general macro writing
rules.

Other than this, this patch looks good to me.

Wei.

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

Re: [Xen-devel] [PATCH for-4.12] tools/tests: Drop obsolete test infrastructure

[Wei Liu]

On Mon, Feb 25, 2019 at 01:16:19PM +, Andrew Cooper wrote:
> The regression/ directory was identified as already broken in 2012 (c/s
> 953953cc5).  The logic is intended to test *.py files in the Xen tree against
> different versions of python, but every identified version is now obsolete.
> 
> Signed-off-by: Andrew Cooper 

Acked-by: Wei Liu 


> ---
> CC: Ian Jackson 
> CC: Wei Liu 
> CC: Juergen Gross 
> 
> For 4.12, this is very safe.  It has been unreachable in the source tree for 7
> years, and was broken before then.

I agree.

Juergen?

Wei.

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

Re: [Xen-devel] [PATCH v4] x86/nmi: correctly check MSB of P6 performance counter MSR in watchdog

[Igor Druzhinin]

On 27/02/2019 10:02, Jan Beulich wrote:
> 
> Reviewed-by: Jan Beulich 
> albeit ...
> 
>> @@ -323,6 +326,15 @@ static void setup_p6_watchdog(unsigned counter)
>>  unsigned int evntsel;
>>  
>>  nmi_perfctr_msr = MSR_P6_PERFCTR(0);
>> +if ( !nmi_p6_event_width )
>> +nmi_p6_event_width = (current_cpu_data.cpuid_level >= 0xa) ?
>> + MASK_EXTR(cpuid_eax(0xa), P6_EVENT_WIDTH_MASK) 
>> :
>> + P6_EVENT_WIDTH_MIN;
>> +if ( !nmi_p6_event_width )
>> +nmi_p6_event_width = P6_EVENT_WIDTH_MIN;
> 
> ... I think this would now better be
> 
> if ( !nmi_p6_event_width && current_cpu_data.cpuid_level >= 0xa )
> nmi_p6_event_width = MASK_EXTR(cpuid_eax(0xa), P6_EVENT_WIDTH_MASK);
> if ( !nmi_p6_event_width )
> nmi_p6_event_width = P6_EVENT_WIDTH_MIN;
> 
> Re-writing of which also mad me notice a hard tab in there. I'd be
> fine making the adjustment while committing, as long as you agree.

Thanks, I also didn't like how it looked eventually. I'll make the same
adjustment to my copy of the patch as well then.

> Btw, considering the combination of subject tag and Cc list I take it
> that you don't intend this to go into 4.12? I ask because generally
> I'd consider this a backporting candidate.

Yes, I didn't intend it to target 4.12 as I don't consider it a serious
issue - we've only seen it on one type of Supermicro machines
(unfortunately, our lab is now almost 50% of them) so far with poor
implementation of ERST. But I wouldn't mind if it was selected as a
candidate for 4.12 and potential backporting.

Igor


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

Re: [Xen-devel] [PATCH 1/1] tools/ocaml: Dup2 /dev/null to stdin in daemonize()

[Wei Liu]

On Wed, Feb 27, 2019 at 10:33:42AM +, Christian Lindig wrote:
> Don't close stdin in daemonize() but dup2 /dev/null instead. This avoids
> fd 0 being reused and potentially written to.
> 
> Signed-off-by: Christian Lindig 

Acked-by: Wei Liu 

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

Re: [Xen-devel] [OSSTEST PATCH] jessie: Drop use of jessie-updates

[Ian Jackson]

Ian Jackson writes ("[OSSTEST PATCH] jessie: Drop use of jessie-updates"):
> The Release file is out of date on our mirror, due to jessie's
> retirement into LTS.

This is causing everything to break.  The self-test flight is `sort
of OK' although it does show what I think are hardware problems.

I'm going to force push this.

Ian.

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

[Xen-devel] [PATCH for-4.12] x86/dom0: propagate PVH vlapic EOIs to hardware

[Roger Pau Monne]

Current check for MSI EIO is missing a special case for PVH Dom0,
which doesn't have a hvm_irq_dpci struct but requires EIOs to be
forwarded to the physical lapic for passed-through devices.

Add a short-circuit to allow EOIs from PVH Dom0 to be propagated.

Signed-off-by: Roger Pau Monné 
---
Cc: Jan Beulich 
Cc: Juergen Gross 
---
 xen/drivers/passthrough/io.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xen/drivers/passthrough/io.c b/xen/drivers/passthrough/io.c
index a6eb8a4336..4290c7c710 100644
--- a/xen/drivers/passthrough/io.c
+++ b/xen/drivers/passthrough/io.c
@@ -869,7 +869,8 @@ static int _hvm_dpci_msi_eoi(struct domain *d,
 
 void hvm_dpci_msi_eoi(struct domain *d, int vector)
 {
-if ( !iommu_enabled || !hvm_domain_irq(d)->dpci )
+if ( !iommu_enabled ||
+ (!hvm_domain_irq(d)->dpci && !is_hardware_domain(d)) )
return;
 
 spin_lock(>event_lock);
-- 
2.17.2 (Apple Git-113)


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

[Xen-devel] [OSSTEST PATCH] jessie: Drop use of jessie-updates

[Ian Jackson]

The Release file is out of date on our mirror, due to jessie's
retirement into LTS.

CC: Juergen Gross 
Signed-off-by: Ian Jackson 
---
 Osstest/Debian.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
index c5dc0e61..59c60d40 100644
--- a/Osstest/Debian.pm
+++ b/Osstest/Debian.pm
@@ -930,10 +930,11 @@ d-i mirror/suite string $suite
 END
 
 $preseed .= <<'END'
-d-i apt-setup/services-select multiselect updates
+d-i apt-setup/services-select multiselect
 END
if $suite =~ m/jessie/;
 # security.d.o CDN seems unreliable right now
+# and jessie-updates is no more
 
 $preseed .= <<"END";
 
-- 
2.11.0


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

Re: [Xen-devel] [PATCH v4 5/6] xen/x86: add PHYSDEVOP_msi_set_enable

[Jan Beulich]

>>> On 07.02.19 at 01:07,  wrote:
> --- a/xen/arch/x86/msi.c
> +++ b/xen/arch/x86/msi.c
> @@ -1474,6 +1474,30 @@ int pci_restore_msi_state(struct pci_dev *pdev)
>  return 0;
>  }
>  
> +int msi_msix_set_enable(struct pci_dev *pdev, int mode, int enable)

unsigned int mode, bool enable

I'm also not happy about the function name. Perhaps simply msi_enable()
or msi_control()?

> +{
> +int ret;
> +
> +ret = xsm_msi_set_enable(XSM_DM_PRIV, pdev->domain,
> + (pdev->seg << 16) | (pdev->bus << 8) | 
> pdev->devfn,
> + mode, enable);
> +if ( ret )
> +return ret;
> +
> +switch ( mode )
> +{
> +case PHYSDEVOP_MSI_SET_ENABLE_MSI:
> +msi_set_enable(pdev, enable);
> +break;
> +
> +case PHYSDEVOP_MSI_SET_ENABLE_MSIX:
> +msix_set_enable(pdev, enable);
> +break;
> +}

What about a call to pci_intx()? And what about invocations for
the wrong device (e.g. MSI-X request for MSI-X incapable device)?

> --- a/xen/arch/x86/physdev.c
> +++ b/xen/arch/x86/physdev.c
> @@ -671,6 +671,30 @@ ret_t do_physdev_op(int cmd, 
> XEN_GUEST_HANDLE_PARAM(void) arg)
>  break;
>  }
>  
> +case PHYSDEVOP_msi_set_enable: {
> +struct physdev_msi_set_enable op;
> +struct pci_dev *pdev;
> +
> +ret = -EFAULT;
> +if ( copy_from_guest(, arg, 1) )
> +break;
> +
> +ret = -EINVAL;
> +if ( op.mode != PHYSDEVOP_MSI_SET_ENABLE_MSI &&
> + op.mode != PHYSDEVOP_MSI_SET_ENABLE_MSIX )
> +break;
> +
> +pcidevs_lock();
> +pdev = pci_get_pdev(op.seg, op.bus, op.devfn);
> +if ( pdev )
> +ret = msi_msix_set_enable(pdev, op.mode, !!op.enable);

Unnecessary !! .

> +else
> +ret = -ENODEV;
> +pcidevs_unlock();
> +break;
> +
> +}

Stray blank line above here.

> --- a/xen/include/public/physdev.h
> +++ b/xen/include/public/physdev.h
> @@ -344,6 +344,21 @@ struct physdev_dbgp_op {
>  typedef struct physdev_dbgp_op physdev_dbgp_op_t;
>  DEFINE_XEN_GUEST_HANDLE(physdev_dbgp_op_t);
>  
> +#define PHYSDEVOP_MSI_SET_ENABLE_MSI  0
> +#define PHYSDEVOP_MSI_SET_ENABLE_MSIX 1
> +
> +#define PHYSDEVOP_msi_set_enable   32
> +struct physdev_msi_set_enable {

Can this please also be something like msi_control?

> +/* IN */
> +uint16_t seg;
> +uint8_t bus;
> +uint8_t devfn;
> +uint8_t mode;
> +uint8_t enable;

"mode" and "enable" don't really make clear which of the two is the
boolean, and which is the operation. I'd anyway prefer a single
flags field with descriptive #define-s, which will also make more
obvious how to extend this if need be.

> --- a/xen/include/xlat.lst
> +++ b/xen/include/xlat.lst
> @@ -106,6 +106,7 @@
>  ?physdev_restore_msi physdev.h
>  ?physdev_set_ioplphysdev.h
>  ?physdev_setup_gsi   physdev.h
> +?physdev_msi_set_enable  physdev.h

Please insert at the alphabetically correct place.

Jan



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

[Xen-devel] [distros-debian-squeeze test] 83674: trouble: blocked/broken

[Platform Team regression test user]

flight 83674 distros-debian-squeeze real [real]
http://osstest.xensource.com/osstest/logs/83674/

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-armhf-pvopsbroken
 build-i386   broken
 build-amd64-pvopsbroken
 build-armhf  broken
 build-amd64  broken
 build-i386-pvops broken
 build-armhf-pvops 4 host-install(4) broken REGR. vs. 75636
 build-armhf   4 host-install(4) broken REGR. vs. 75636
 build-i3864 host-install(4) broken REGR. vs. 75636
 build-amd64-pvops 4 host-install(4) broken REGR. vs. 75636
 build-i386-pvops  4 host-install(4) broken REGR. vs. 75636
 build-amd64   4 host-install(4) broken REGR. vs. 75636
 build-armhf-pvops 3 syslog-serverrunning
 build-armhf   3 syslog-serverrunning

Tests which did not succeed, but are not blocking:
 test-amd64-i386-i386-squeeze-netboot-pygrub  1 build-check(1)  blocked n/a
 test-amd64-amd64-i386-squeeze-netboot-pygrub  1 build-check(1) blocked n/a
 test-amd64-i386-amd64-squeeze-netboot-pygrub  1 build-check(1) blocked n/a
 test-amd64-amd64-amd64-squeeze-netboot-pygrub  1 build-check(1)blocked n/a
 build-armhf-pvops 5 capture-logs   broken blocked in 75636
 build-armhf   5 capture-logs   broken blocked in 75636

baseline version:
 flight   75636

jobs:
 build-amd64  broken  
 build-armhf  broken  
 build-i386   broken  
 build-amd64-pvopsbroken  
 build-armhf-pvopsbroken  
 build-i386-pvops broken  
 test-amd64-amd64-amd64-squeeze-netboot-pygrubblocked 
 test-amd64-i386-amd64-squeeze-netboot-pygrub blocked 
 test-amd64-amd64-i386-squeeze-netboot-pygrub blocked 
 test-amd64-i386-i386-squeeze-netboot-pygrub  blocked 



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.xensource.com/osstest/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.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v6.1 3/5] p2m: change write_p2m_entry to return an error code

[Roger Pau Monne]

This is in preparation for also changing p2m_entry_modify to return an
error code.

No functional change intended.

Signed-off-by: Roger Pau Monné 
---
Cc: George Dunlap 
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Wei Liu 
Cc: Tim Deegan 
---
Changes since v6:
 - Fix line wrapping in p2m_next_level.

Changes since v5:
 - Return -EOPNOTSUPP from _write_p2m_entry.
 - Use an error label in p2m_next_level.

Changes since v4:
 - Handle errors in loops to avoid overwriting the variable
   containing the error code in non-debug builds.
 - Change error handling in p2m_next_level so it's done in a single
   place.

Changes since v3:
 - Use asserts instead of bugs to check return code from
   write_p2m_entry from callers that don't support or expect
   write_p2m_entry to fail.

Changes since v2:
 - New in this version.
---
 xen/arch/x86/mm/hap/hap.c|  4 +-
 xen/arch/x86/mm/hap/nested_hap.c |  4 +-
 xen/arch/x86/mm/p2m-pt.c | 73 ++--
 xen/arch/x86/mm/paging.c | 12 --
 xen/arch/x86/mm/shadow/common.c  |  4 +-
 xen/arch/x86/mm/shadow/none.c|  7 +--
 xen/arch/x86/mm/shadow/private.h |  6 +--
 xen/include/asm-x86/p2m.h|  4 +-
 xen/include/asm-x86/paging.h |  8 ++--
 9 files changed, 89 insertions(+), 33 deletions(-)

diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
index 2db7f2c04a..fdf77c59a5 100644
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -708,7 +708,7 @@ static void hap_update_paging_modes(struct vcpu *v)
 put_gfn(d, cr3_gfn);
 }
 
-static void
+static int
 hap_write_p2m_entry(struct p2m_domain *p2m, unsigned long gfn, l1_pgentry_t *p,
 l1_pgentry_t new, unsigned int level)
 {
@@ -746,6 +746,8 @@ hap_write_p2m_entry(struct p2m_domain *p2m, unsigned long 
gfn, l1_pgentry_t *p,
 
 if ( flush_nestedp2m )
 p2m_flush_nestedp2m(d);
+
+return 0;
 }
 
 static unsigned long hap_gva_to_gfn_real_mode(
diff --git a/xen/arch/x86/mm/hap/nested_hap.c b/xen/arch/x86/mm/hap/nested_hap.c
index d2a07a5c79..abe5958a52 100644
--- a/xen/arch/x86/mm/hap/nested_hap.c
+++ b/xen/arch/x86/mm/hap/nested_hap.c
@@ -71,7 +71,7 @@
 /*NESTED VIRT P2M FUNCTIONS */
 //
 
-void
+int
 nestedp2m_write_p2m_entry(struct p2m_domain *p2m, unsigned long gfn,
 l1_pgentry_t *p, l1_pgentry_t new, unsigned int level)
 {
@@ -87,6 +87,8 @@ nestedp2m_write_p2m_entry(struct p2m_domain *p2m, unsigned 
long gfn,
 flush_tlb_mask(p2m->dirty_cpumask);
 
 paging_unlock(d);
+
+return 0;
 }
 
 //
diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c
index 04e9d81cf6..d0f35d8b47 100644
--- a/xen/arch/x86/mm/p2m-pt.c
+++ b/xen/arch/x86/mm/p2m-pt.c
@@ -184,6 +184,8 @@ p2m_next_level(struct p2m_domain *p2m, void **table,
 l1_pgentry_t *p2m_entry, new_entry;
 void *next;
 unsigned int flags;
+int rc;
+mfn_t mfn;
 
 if ( !(p2m_entry = p2m_find_entry(*table, gfn_remainder, gfn,
   shift, max)) )
@@ -194,7 +196,7 @@ p2m_next_level(struct p2m_domain *p2m, void **table,
 /* PoD/paging: Not present doesn't imply empty. */
 if ( !flags )
 {
-mfn_t mfn = p2m_alloc_ptp(p2m, level);
+mfn = p2m_alloc_ptp(p2m, level);
 
 if ( mfn_eq(mfn, INVALID_MFN) )
 return -ENOMEM;
@@ -202,13 +204,14 @@ p2m_next_level(struct p2m_domain *p2m, void **table,
 new_entry = l1e_from_mfn(mfn, P2M_BASE_FLAGS | _PAGE_RW);
 
 p2m_add_iommu_flags(_entry, level, 
IOMMUF_readable|IOMMUF_writable);
-p2m->write_p2m_entry(p2m, gfn, p2m_entry, new_entry, level + 1);
+rc = p2m->write_p2m_entry(p2m, gfn, p2m_entry, new_entry, level + 1);
+if ( rc )
+goto error;
 }
 else if ( flags & _PAGE_PSE )
 {
 /* Split superpages pages into smaller ones. */
 unsigned long pfn = l1e_get_pfn(*p2m_entry);
-mfn_t mfn;
 l1_pgentry_t *l1_entry;
 unsigned int i;
 
@@ -250,14 +253,21 @@ p2m_next_level(struct p2m_domain *p2m, void **table,
 {
 new_entry = l1e_from_pfn(pfn | (i << ((level - 1) * 
PAGETABLE_ORDER)),
  flags);
-p2m->write_p2m_entry(p2m, gfn, l1_entry + i, new_entry, level);
+rc = p2m->write_p2m_entry(p2m, gfn, l1_entry + i, new_entry, 
level);
+if ( rc )
+{
+unmap_domain_page(l1_entry);
+goto error;
+}
 }
 
 unmap_domain_page(l1_entry);
 
 new_entry = l1e_from_mfn(mfn, P2M_BASE_FLAGS | _PAGE_RW);
 p2m_add_iommu_flags(_entry, level, 
IOMMUF_readable|IOMMUF_writable);
-p2m->write_p2m_entry(p2m, gfn, p2m_entry, new_entry, level + 1);
+rc = p2m->write_p2m_entry(p2m, gfn, p2m_entry, new_entry, level + 1);
+if ( rc )
+goto error;

Re: [Xen-devel] [PATCH for-4.12] x86: Improve the efficiency of domain_relinquish_resources()

[Julien Grall]

Hi Andrew,

On 2/21/19 12:22 PM, Andrew Cooper wrote:

pci_release_devices() takes the global PCI lock.  Once pci_release_devices()
has completed, it will be called redundantly each time paging_teardown() and
vcpu_destroy_pagetables() continue.

This is liable to be millions of times for a reasonably sized guest, and is a
serialising bottleneck now that domain_kill() can be run concurrently on
different domains.

Instead of propagating the opencoding of the relinquish state machine, take
the opportunity to clean it up.

Leave a proper set of comments explaining that domain_relinquish_resources()
implements a co-routine.  Introduce a documented PROGRESS() macro to avoid
latent bugs such as the RELMEM_xen case, and make the new PROG_* states
private to domain_relinquish_resources().

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Wei Liu 
CC: Roger Pau Monné 
CC: Juergen Gross 
CC: Stefano Stabellini 
CC: Julien Grall 

So I know Xen 4.12 isn't going to crash and burn without this change, but I
also can't un-see the unnecessary global PCI lock contention.  In terms of
risk, this is extremely low - this function has complete coverage in testing,
and its behaviour isn't changing dramatically.

ARM: There are no problems, latent or otherwise, with your version of
domain_relinquish_resources(), but I'd recommend the same cleanup in due
course.


I will add in my todo list of cleanup!

Cheers,

--
Julien Grall

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

Re: [Xen-devel] xen/evtchn and forced threaded irq

[Julien Grall]

Hi,

On 2/26/19 11:02 AM, Roger Pau Monné wrote:

On Tue, Feb 26, 2019 at 10:26:21AM +, Julien Grall wrote:

On 26/02/2019 10:17, Roger Pau Monné wrote:

On Tue, Feb 26, 2019 at 10:03:38AM +, Julien Grall wrote:

Hi Roger,

On 26/02/2019 09:44, Roger Pau Monné wrote:

On Tue, Feb 26, 2019 at 09:30:07AM +, Andrew Cooper wrote:

On 26/02/2019 09:14, Roger Pau Monné wrote:

On Mon, Feb 25, 2019 at 01:55:42PM +, Julien Grall wrote:

Hi Oleksandr,

On 25/02/2019 13:24, Oleksandr Andrushchenko wrote:

On 2/22/19 3:33 PM, Julien Grall wrote:

Hi,

On 22/02/2019 12:38, Oleksandr Andrushchenko wrote:

On 2/20/19 10:46 PM, Julien Grall wrote:

Discussing with my team, a solution that came up would be to
introduce one atomic field per event to record the number of
event received. I will explore that solution tomorrow.

How will this help if events have some payload?

What payload? The event channel does not carry any payload. It only
notify you that something happen. Then this is up to the user to
decide what to you with it.

Sorry, I was probably not precise enough. I mean that an event might have
associated payload in the ring buffer, for example [1]. So, counting events
may help somehow, but the ring's data may still be lost

   From my understanding of event channels are edge interrupts. By definition,

IMO event channels are active high level interrupts.

Let's take into account the following situation: you have an event
channel masked and the event channel pending bit (akin to the line on
bare metal) goes from low to high (0 -> 1), then you unmask the
interrupt and you get an event injected. If it was an edge interrupt
you wont get an event injected after unmasking, because you would
have lost the edge. I think the problem here is that Linux treats
event channels as edge interrupts, when they are actually level.


Event channels are edge interrupts.  There are several very subtle bugs
to be had by software which treats them as line interrupts.

Most critically, if you fail to ack them, rebind them to a new vcpu, and
reenable interrupts, you don't get a new interrupt notification.  This
was the source of a 4 month bug when XenServer was moving from
classic-xen to PVOps where using irqbalance would cause dom0 to
occasionally lose interrupts.


I would argue that you need to mask them first, rebind to a new vcpu
and unmask, and then you will get an interrupt notification, or this
should be fixed in Xen to work as you expect: trigger an interrupt
notification when moving an asserted event channel between CPUs.

Is there any document that describes how such non trivial things (like
moving between CPUs) work for event/level interrupts?

Maybe I'm being obtuse, but from the example I gave above it's quite
clear to me event channels don't get triggered based on edge changes,
but rather on the line level.


Your example above is not enough to give the semantics of level. You would
only use the MASK bit if your interrupt handler is threaded to avoid the
interrupt coming up again.

So if you remove the mask from the equation, then the interrupt flow should be:

1) handle interrupt
2) EOI


This is bogus if you don't mask the interrupt source. You should
instead do

1) EOI
2) Handle interrupt

And loop over this.

So that's not a level semantics. It is a edge one :). In the level case, you
would clear the state once you are done with the interrupt.

Also, it would be ACK and not EOI.


For level triggered interrupts you have to somehow signal the device
to stop asserting the line, which doesn't happen for Xen devices
because they just signal interrupts to Xen, but don't have a way to
keep event channels asserted, so I agree that this is different from
traditional level interrupts because devices using event channels
don't have a way to keep lines asserted.

I guess the most similar native interrupt is MSI with masking
support?


I don't know enough about MSI with masking support to be able to draw a 
comparison :).


The flow I have been suggested to re-use in Linux is 
handle_fasteoi_ack_irq. I haven't yet had time to have a try at it.


Cheers,

--
Julien Grall

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

[Xen-devel] [PATCH v6 2/5] x86/mm: split p2m ioreq server pages special handling into helper

[Roger Pau Monne]

So that it can be shared by both ept, npt and shadow code, instead of
duplicating it.

No change in functionality intended.

Signed-off-by: Roger Pau Monné 
Reviewed-by: Paul Durrant 
Reviewed-by: George Dunlap 
Reviewed-by: Kevin Tian 
---
Cc: George Dunlap 
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Wei Liu 
Cc: Jun Nakajima 
Cc: Kevin Tian 
Cc: Paul Durrant 
Cc: Tim Deegan 
---
Changes since v4:
 - Remove the p2m_get_hostp2m from callers of p2m_entry_modify.

Changes since v1:
 - Remove unused p2mt_old from p2m_pt_set_entry.
---
 xen/arch/x86/mm/hap/hap.c   |  3 ++
 xen/arch/x86/mm/p2m-ept.c   | 55 +++--
 xen/arch/x86/mm/p2m-pt.c| 24 --
 xen/arch/x86/mm/shadow/common.c |  3 ++
 xen/include/asm-x86/p2m.h   | 32 +++
 5 files changed, 56 insertions(+), 61 deletions(-)

diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
index 28fe48d158..2db7f2c04a 100644
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -735,6 +735,9 @@ hap_write_p2m_entry(struct p2m_domain *p2m, unsigned long 
gfn, l1_pgentry_t *p,
 && perms_strictly_increased(old_flags, l1e_get_flags(new)) );
 }
 
+p2m_entry_modify(p2m, p2m_flags_to_type(l1e_get_flags(new)),
+ p2m_flags_to_type(old_flags), level);
+
 safe_write_pte(p, new);
 if ( old_flags & _PAGE_PRESENT )
 flush_tlb_mask(d->dirty_cpumask);
diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
index bb562607f7..0ece6608cb 100644
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -46,7 +46,8 @@ static inline bool_t is_epte_valid(ept_entry_t *e)
 }
 
 /* returns : 0 for success, -errno otherwise */
-static int atomic_write_ept_entry(ept_entry_t *entryptr, ept_entry_t new,
+static int atomic_write_ept_entry(struct p2m_domain *p2m,
+  ept_entry_t *entryptr, ept_entry_t new,
   int level)
 {
 int rc;
@@ -89,6 +90,8 @@ static int atomic_write_ept_entry(ept_entry_t *entryptr, 
ept_entry_t new,
 if ( unlikely(p2m_is_foreign(entryptr->sa_p2mt)) && check_foreign )
 oldmfn = entryptr->mfn;
 
+p2m_entry_modify(p2m, new.sa_p2mt, entryptr->sa_p2mt, level);
+
 write_atomic(>epte, new.epte);
 
 if ( unlikely(oldmfn != mfn_x(INVALID_MFN)) )
@@ -390,7 +393,8 @@ static int ept_next_level(struct p2m_domain *p2m, bool_t 
read_only,
  * present entries in the given page table, optionally marking the entries
  * also for their subtrees needing P2M type re-calculation.
  */
-static bool_t ept_invalidate_emt(mfn_t mfn, bool_t recalc, int level)
+static bool_t ept_invalidate_emt(struct p2m_domain *p2m, mfn_t mfn,
+ bool_t recalc, int level)
 {
 int rc;
 ept_entry_t *epte = map_domain_page(mfn);
@@ -408,7 +412,7 @@ static bool_t ept_invalidate_emt(mfn_t mfn, bool_t recalc, 
int level)
 e.emt = MTRR_NUM_TYPES;
 if ( recalc )
 e.recalc = 1;
-rc = atomic_write_ept_entry([i], e, level);
+rc = atomic_write_ept_entry(p2m, [i], e, level);
 ASSERT(rc == 0);
 changed = 1;
 }
@@ -459,7 +463,7 @@ static int ept_invalidate_emt_range(struct p2m_domain *p2m,
 rc = -ENOMEM;
 goto out;
 }
-wrc = atomic_write_ept_entry([index], split_ept_entry, i);
+wrc = atomic_write_ept_entry(p2m, [index], split_ept_entry, i);
 ASSERT(wrc == 0);
 
 for ( ; i > target; --i )
@@ -479,7 +483,7 @@ static int ept_invalidate_emt_range(struct p2m_domain *p2m,
 {
 e.emt = MTRR_NUM_TYPES;
 e.recalc = 1;
-wrc = atomic_write_ept_entry([index], e, target);
+wrc = atomic_write_ept_entry(p2m, [index], e, target);
 ASSERT(wrc == 0);
 rc = 1;
 }
@@ -549,17 +553,11 @@ static int resolve_misconfig(struct p2m_domain *p2m, 
unsigned long gfn)
 nt = p2m_recalc_type(e.recalc, e.sa_p2mt, p2m, gfn + i);
 if ( nt != e.sa_p2mt )
 {
-if ( e.sa_p2mt == p2m_ioreq_server )
-{
-ASSERT(p2m->ioreq.entry_count > 0);
-p2m->ioreq.entry_count--;
-}
-
 e.sa_p2mt = nt;
 ept_p2m_type_to_flags(p2m, , e.sa_p2mt, e.access);
 }
 e.recalc = 0;
-wrc = atomic_write_ept_entry([i], e, level);
+wrc = atomic_write_ept_entry(p2m, [i], e, level);
 ASSERT(wrc == 0);
 }
 }
@@ -595,7 +593,7 @@ static int resolve_misconfig(struct p2m_domain *p2m, 
unsigned long gfn)
 {
 if ( ept_split_super_page(p2m, , level, level - 1) )
 {
-wrc = 

[Xen-devel] [PATCH v6 1/5] x86/p2m: pass the p2m to write_p2m_entry handlers

[Roger Pau Monne]

Current callers pass the p2m to paging_write_p2m_entry, but the
implementation specific handlers of the write_p2m_entry hook instead
of a p2m get a domain struct due to the handling done in
paging_write_p2m_entry.

Change the code so that the implementations of write_p2m_entry take a
p2m instead of a domain.

This is a non-functional change, but will be used by follow up
patches.

Signed-off-by: Roger Pau Monné 
Reviewed-by: Wei Liu 
Reviewed-by: George Dunlap 
---
Cc: George Dunlap 
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Wei Liu 
Cc: Tim Deegan 
---
Changes since v4:
 - New in this version.
---
 xen/arch/x86/mm/hap/hap.c| 3 ++-
 xen/arch/x86/mm/paging.c | 2 +-
 xen/arch/x86/mm/shadow/common.c  | 4 +++-
 xen/arch/x86/mm/shadow/none.c| 2 +-
 xen/arch/x86/mm/shadow/private.h | 2 +-
 xen/include/asm-x86/paging.h | 3 ++-
 6 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
index 3d651b94c3..28fe48d158 100644
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -709,9 +709,10 @@ static void hap_update_paging_modes(struct vcpu *v)
 }
 
 static void
-hap_write_p2m_entry(struct domain *d, unsigned long gfn, l1_pgentry_t *p,
+hap_write_p2m_entry(struct p2m_domain *p2m, unsigned long gfn, l1_pgentry_t *p,
 l1_pgentry_t new, unsigned int level)
 {
+struct domain *d = p2m->domain;
 uint32_t old_flags;
 bool_t flush_nestedp2m = 0;
 
diff --git a/xen/arch/x86/mm/paging.c b/xen/arch/x86/mm/paging.c
index d5836eb688..e6ed3006fe 100644
--- a/xen/arch/x86/mm/paging.c
+++ b/xen/arch/x86/mm/paging.c
@@ -941,7 +941,7 @@ void paging_write_p2m_entry(struct p2m_domain *p2m, 
unsigned long gfn,
 if ( v->domain != d )
 v = d->vcpu ? d->vcpu[0] : NULL;
 if ( likely(v && paging_mode_enabled(d) && paging_get_hostmode(v) != NULL) 
)
-paging_get_hostmode(v)->write_p2m_entry(d, gfn, p, new, level);
+paging_get_hostmode(v)->write_p2m_entry(p2m, gfn, p, new, level);
 else
 safe_write_pte(p, new);
 }
diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c
index 07840ff727..6c67ef4996 100644
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -3177,10 +3177,12 @@ static void sh_unshadow_for_p2m_change(struct domain 
*d, unsigned long gfn,
 }
 
 void
-shadow_write_p2m_entry(struct domain *d, unsigned long gfn,
+shadow_write_p2m_entry(struct p2m_domain *p2m, unsigned long gfn,
l1_pgentry_t *p, l1_pgentry_t new,
unsigned int level)
 {
+struct domain *d = p2m->domain;
+
 paging_lock(d);
 
 /* If there are any shadows, update them.  But if shadow_teardown()
diff --git a/xen/arch/x86/mm/shadow/none.c b/xen/arch/x86/mm/shadow/none.c
index 4de645a433..316002771d 100644
--- a/xen/arch/x86/mm/shadow/none.c
+++ b/xen/arch/x86/mm/shadow/none.c
@@ -60,7 +60,7 @@ static void _update_paging_modes(struct vcpu *v)
 ASSERT_UNREACHABLE();
 }
 
-static void _write_p2m_entry(struct domain *d, unsigned long gfn,
+static void _write_p2m_entry(struct p2m_domain *p2m, unsigned long gfn,
  l1_pgentry_t *p, l1_pgentry_t new,
  unsigned int level)
 {
diff --git a/xen/arch/x86/mm/shadow/private.h b/xen/arch/x86/mm/shadow/private.h
index e8ed7ac714..0aaed1edfc 100644
--- a/xen/arch/x86/mm/shadow/private.h
+++ b/xen/arch/x86/mm/shadow/private.h
@@ -372,7 +372,7 @@ extern int sh_remove_write_access(struct domain *d, mfn_t 
readonly_mfn,
   unsigned long fault_addr);
 
 /* Functions that atomically write PT/P2M entries and update state */
-void shadow_write_p2m_entry(struct domain *d, unsigned long gfn,
+void shadow_write_p2m_entry(struct p2m_domain *p2m, unsigned long gfn,
 l1_pgentry_t *p, l1_pgentry_t new,
 unsigned int level);
 
diff --git a/xen/include/asm-x86/paging.h b/xen/include/asm-x86/paging.h
index fdcc22844b..7ec09d7b11 100644
--- a/xen/include/asm-x86/paging.h
+++ b/xen/include/asm-x86/paging.h
@@ -124,7 +124,8 @@ struct paging_mode {
 void  (*update_cr3)(struct vcpu *v, int do_locking,
 bool noflush);
 void  (*update_paging_modes   )(struct vcpu *v);
-void  (*write_p2m_entry   )(struct domain *d, unsigned long 
gfn,
+void  (*write_p2m_entry   )(struct p2m_domain *p2m,
+unsigned long gfn,
 l1_pgentry_t *p, l1_pgentry_t new,
 unsigned int level);
 
-- 
2.17.2 (Apple Git-113)


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

[Xen-devel] [PATCH v6 5/5] npt/shadow: allow getting foreign page table entries

[Roger Pau Monne]

Current npt and shadow code to get an entry will always return
INVALID_MFN for foreign entries. Allow to return the entry mfn for
foreign entries, like it's done for grant table entries.

Signed-off-by: Roger Pau Monné 
Reviewed-by: Jan Beulich 
---
Cc: George Dunlap 
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Wei Liu 
---
Changes since v2:
 - Use p2m_is_any_ram.
---
 xen/arch/x86/mm/p2m-pt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c
index e62bafcfb7..cafc9f299b 100644
--- a/xen/arch/x86/mm/p2m-pt.c
+++ b/xen/arch/x86/mm/p2m-pt.c
@@ -903,8 +903,8 @@ pod_retry_l1:
 *t = p2m_recalc_type(recalc || _needs_recalc(flags), l1t, p2m, gfn);
 unmap_domain_page(l1e);
 
-ASSERT(mfn_valid(mfn) || !p2m_is_ram(*t) || p2m_is_paging(*t));
-return (p2m_is_valid(*t) || p2m_is_grant(*t)) ? mfn : INVALID_MFN;
+ASSERT(mfn_valid(mfn) || !p2m_is_any_ram(*t) || p2m_is_paging(*t));
+return (p2m_is_valid(*t) || p2m_is_any_ram(*t)) ? mfn : INVALID_MFN;
 }
 
 static void p2m_pt_change_entry_type_global(struct p2m_domain *p2m,
-- 
2.17.2 (Apple Git-113)


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

[Xen-devel] [PATCH v6 4/5] x86/mm: handle foreign mappings in p2m_entry_modify

[Roger Pau Monne]

So that the specific handling can be removed from
atomic_write_ept_entry and be shared with npt and shadow code.

This commit also removes the check that prevent non-ept PVH dom0 from
mapping foreign pages.

Signed-off-by: Roger Pau Monné 
Reviewed-by: Kevin Tian 
---
Cc: George Dunlap 
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Wei Liu 
Cc: Jun Nakajima 
Cc: Kevin Tian 
Cc: Tim Deegan 
---
Changes since v3:
 - Replace the mfn_valid BUG_ONs with an assert & return.

Changes since v2:
 - Return an error code from p2m_entry_modify and propagate it to the
   callers.

Changes since v1:
 - Simply code since mfn_to_page cannot return NULL.
 - Check if the mfn is valid before getting/dropping the page reference.
 - Use BUG_ON instead of ASSERTs, since getting the reference counting
   wrong is more dangerous than a DoS.
---
 xen/arch/x86/mm/hap/hap.c   | 11 +--
 xen/arch/x86/mm/p2m-ept.c   | 55 +++--
 xen/arch/x86/mm/p2m-pt.c|  7 -
 xen/arch/x86/mm/shadow/common.c | 11 +--
 xen/include/asm-x86/p2m.h   | 34 +---
 5 files changed, 53 insertions(+), 65 deletions(-)

diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
index fdf77c59a5..412a442b6a 100644
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -715,6 +715,7 @@ hap_write_p2m_entry(struct p2m_domain *p2m, unsigned long 
gfn, l1_pgentry_t *p,
 struct domain *d = p2m->domain;
 uint32_t old_flags;
 bool_t flush_nestedp2m = 0;
+int rc;
 
 /* We know always use the host p2m here, regardless if the vcpu
  * is in host or guest mode. The vcpu can be in guest mode by
@@ -735,8 +736,14 @@ hap_write_p2m_entry(struct p2m_domain *p2m, unsigned long 
gfn, l1_pgentry_t *p,
 && perms_strictly_increased(old_flags, l1e_get_flags(new)) );
 }
 
-p2m_entry_modify(p2m, p2m_flags_to_type(l1e_get_flags(new)),
- p2m_flags_to_type(old_flags), level);
+rc = p2m_entry_modify(p2m, p2m_flags_to_type(l1e_get_flags(new)),
+  p2m_flags_to_type(old_flags), l1e_get_mfn(new),
+  l1e_get_mfn(*p), level);
+if ( rc )
+{
+paging_unlock(d);
+return rc;
+}
 
 safe_write_pte(p, new);
 if ( old_flags & _PAGE_PRESENT )
diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
index 0ece6608cb..e3044bee2e 100644
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -50,60 +50,15 @@ static int atomic_write_ept_entry(struct p2m_domain *p2m,
   ept_entry_t *entryptr, ept_entry_t new,
   int level)
 {
-int rc;
-unsigned long oldmfn = mfn_x(INVALID_MFN);
-bool_t check_foreign = (new.mfn != entryptr->mfn ||
-new.sa_p2mt != entryptr->sa_p2mt);
-
-if ( level )
-{
-ASSERT(!is_epte_superpage() || !p2m_is_foreign(new.sa_p2mt));
-write_atomic(>epte, new.epte);
-return 0;
-}
-
-if ( unlikely(p2m_is_foreign(new.sa_p2mt)) )
-{
-rc = -EINVAL;
-if ( !is_epte_present() )
-goto out;
-
-if ( check_foreign )
-{
-struct domain *fdom;
-
-if ( !mfn_valid(_mfn(new.mfn)) )
-goto out;
-
-rc = -ESRCH;
-fdom = page_get_owner(mfn_to_page(_mfn(new.mfn)));
-if ( fdom == NULL )
-goto out;
+int rc = p2m_entry_modify(p2m, new.sa_p2mt, entryptr->sa_p2mt,
+  _mfn(new.mfn), _mfn(entryptr->mfn), level);
 
-/* get refcount on the page */
-rc = -EBUSY;
-if ( !get_page(mfn_to_page(_mfn(new.mfn)), fdom) )
-goto out;
-}
-}
-
-if ( unlikely(p2m_is_foreign(entryptr->sa_p2mt)) && check_foreign )
-oldmfn = entryptr->mfn;
-
-p2m_entry_modify(p2m, new.sa_p2mt, entryptr->sa_p2mt, level);
+if ( rc )
+return rc;
 
 write_atomic(>epte, new.epte);
 
-if ( unlikely(oldmfn != mfn_x(INVALID_MFN)) )
-put_page(mfn_to_page(_mfn(oldmfn)));
-
-rc = 0;
-
- out:
-if ( rc )
-gdprintk(XENLOG_ERR, "epte o:%"PRIx64" n:%"PRIx64" rc:%d\n",
- entryptr->epte, new.epte, rc);
-return rc;
+return 0;
 }
 
 static void ept_p2m_type_to_flags(struct p2m_domain *p2m, ept_entry_t *entry,
diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c
index 4a531fdf9d..e62bafcfb7 100644
--- a/xen/arch/x86/mm/p2m-pt.c
+++ b/xen/arch/x86/mm/p2m-pt.c
@@ -574,13 +574,6 @@ p2m_pt_set_entry(struct p2m_domain *p2m, gfn_t gfn_, mfn_t 
mfn,
 __trace_var(TRC_MEM_SET_P2M_ENTRY, 0, sizeof(t), );
 }
 
-if ( unlikely(p2m_is_foreign(p2mt)) )
-{
-/* hvm fixme: foreign types are only supported on ept at present */
-gdprintk(XENLOG_WARNING, "Unimplemented foreign p2m type.\n");
-return -EINVAL;
-}
-
 /* Carry out any 

[Xen-devel] [PATCH v6 0/5] pvh/dom0/shadow/amd fixes

[Roger Pau Monne]

The remaining set of patches contain changes to the p2m code that could
affect HVM guests. Note that without those changes a PVH dom0 running on
AMD hardware will be unable to create guests. Overall the patches are a
nice cleanup to the handling of p2m_ioreq_server and p2m_map_foreign
types IMO.

The series can also be found at:

git://xenbits.xen.org/people/royger/xen.git fixes-v6

Thanks, Roger.

Roger Pau Monne (5):
  x86/p2m: pass the p2m to write_p2m_entry handlers
  x86/mm: split p2m ioreq server pages special handling into helper
  p2m: change write_p2m_entry to return an error code
  x86/mm: handle foreign mappings in p2m_entry_modify
  npt/shadow: allow getting foreign page table entries

 xen/arch/x86/mm/hap/hap.c|  17 -
 xen/arch/x86/mm/hap/nested_hap.c |   4 +-
 xen/arch/x86/mm/p2m-ept.c| 106 ++---
 xen/arch/x86/mm/p2m-pt.c | 112 ++-
 xen/arch/x86/mm/paging.c |  12 ++--
 xen/arch/x86/mm/shadow/common.c  |  18 -
 xen/arch/x86/mm/shadow/none.c|   7 +-
 xen/arch/x86/mm/shadow/private.h |   6 +-
 xen/include/asm-x86/p2m.h|  62 -
 xen/include/asm-x86/paging.h |   9 +--
 10 files changed, 199 insertions(+), 154 deletions(-)

-- 
2.17.2 (Apple Git-113)


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

[Xen-devel] [PATCH v6 3/5] p2m: change write_p2m_entry to return an error code

[Roger Pau Monne]

This is in preparation for also changing p2m_entry_modify to return an
error code.

No functional change intended.

Signed-off-by: Roger Pau Monné 
---
Cc: George Dunlap 
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Wei Liu 
Cc: Tim Deegan 
---
Changes since v5:
 - Return -EOPNOTSUPP from _write_p2m_entry.
 - Use an error label in p2m_next_level.

Changes since v4:
 - Handle errors in loops to avoid overwriting the variable
   containing the error code in non-debug builds.
 - Change error handling in p2m_next_level so it's done in a single
   place.

Changes since v3:
 - Use asserts instead of bugs to check return code from
   write_p2m_entry from callers that don't support or expect
   write_p2m_entry to fail.

Changes since v2:
 - New in this version.
---
 xen/arch/x86/mm/hap/hap.c|  4 +-
 xen/arch/x86/mm/hap/nested_hap.c |  4 +-
 xen/arch/x86/mm/p2m-pt.c | 77 +---
 xen/arch/x86/mm/paging.c | 12 +++--
 xen/arch/x86/mm/shadow/common.c  |  4 +-
 xen/arch/x86/mm/shadow/none.c|  7 +--
 xen/arch/x86/mm/shadow/private.h |  6 +--
 xen/include/asm-x86/p2m.h|  4 +-
 xen/include/asm-x86/paging.h |  8 ++--
 9 files changed, 92 insertions(+), 34 deletions(-)

diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
index 2db7f2c04a..fdf77c59a5 100644
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -708,7 +708,7 @@ static void hap_update_paging_modes(struct vcpu *v)
 put_gfn(d, cr3_gfn);
 }
 
-static void
+static int
 hap_write_p2m_entry(struct p2m_domain *p2m, unsigned long gfn, l1_pgentry_t *p,
 l1_pgentry_t new, unsigned int level)
 {
@@ -746,6 +746,8 @@ hap_write_p2m_entry(struct p2m_domain *p2m, unsigned long 
gfn, l1_pgentry_t *p,
 
 if ( flush_nestedp2m )
 p2m_flush_nestedp2m(d);
+
+return 0;
 }
 
 static unsigned long hap_gva_to_gfn_real_mode(
diff --git a/xen/arch/x86/mm/hap/nested_hap.c b/xen/arch/x86/mm/hap/nested_hap.c
index d2a07a5c79..abe5958a52 100644
--- a/xen/arch/x86/mm/hap/nested_hap.c
+++ b/xen/arch/x86/mm/hap/nested_hap.c
@@ -71,7 +71,7 @@
 /*NESTED VIRT P2M FUNCTIONS */
 //
 
-void
+int
 nestedp2m_write_p2m_entry(struct p2m_domain *p2m, unsigned long gfn,
 l1_pgentry_t *p, l1_pgentry_t new, unsigned int level)
 {
@@ -87,6 +87,8 @@ nestedp2m_write_p2m_entry(struct p2m_domain *p2m, unsigned 
long gfn,
 flush_tlb_mask(p2m->dirty_cpumask);
 
 paging_unlock(d);
+
+return 0;
 }
 
 //
diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c
index 04e9d81cf6..4a531fdf9d 100644
--- a/xen/arch/x86/mm/p2m-pt.c
+++ b/xen/arch/x86/mm/p2m-pt.c
@@ -184,6 +184,8 @@ p2m_next_level(struct p2m_domain *p2m, void **table,
 l1_pgentry_t *p2m_entry, new_entry;
 void *next;
 unsigned int flags;
+int rc;
+mfn_t mfn;
 
 if ( !(p2m_entry = p2m_find_entry(*table, gfn_remainder, gfn,
   shift, max)) )
@@ -194,7 +196,7 @@ p2m_next_level(struct p2m_domain *p2m, void **table,
 /* PoD/paging: Not present doesn't imply empty. */
 if ( !flags )
 {
-mfn_t mfn = p2m_alloc_ptp(p2m, level);
+mfn = p2m_alloc_ptp(p2m, level);
 
 if ( mfn_eq(mfn, INVALID_MFN) )
 return -ENOMEM;
@@ -202,13 +204,14 @@ p2m_next_level(struct p2m_domain *p2m, void **table,
 new_entry = l1e_from_mfn(mfn, P2M_BASE_FLAGS | _PAGE_RW);
 
 p2m_add_iommu_flags(_entry, level, 
IOMMUF_readable|IOMMUF_writable);
-p2m->write_p2m_entry(p2m, gfn, p2m_entry, new_entry, level + 1);
+rc = p2m->write_p2m_entry(p2m, gfn, p2m_entry, new_entry, level + 1);
+if ( rc )
+goto error;
 }
 else if ( flags & _PAGE_PSE )
 {
 /* Split superpages pages into smaller ones. */
 unsigned long pfn = l1e_get_pfn(*p2m_entry);
-mfn_t mfn;
 l1_pgentry_t *l1_entry;
 unsigned int i;
 
@@ -250,14 +253,23 @@ p2m_next_level(struct p2m_domain *p2m, void **table,
 {
 new_entry = l1e_from_pfn(pfn | (i << ((level - 1) * 
PAGETABLE_ORDER)),
  flags);
-p2m->write_p2m_entry(p2m, gfn, l1_entry + i, new_entry, level);
+rc = p2m->write_p2m_entry(p2m, gfn, l1_entry + i, new_entry, 
level);
+if ( rc )
+{
+unmap_domain_page(l1_entry);
+goto error;
+}
 }
 
 unmap_domain_page(l1_entry);
 
 new_entry = l1e_from_mfn(mfn, P2M_BASE_FLAGS | _PAGE_RW);
-p2m_add_iommu_flags(_entry, level, 
IOMMUF_readable|IOMMUF_writable);
-p2m->write_p2m_entry(p2m, gfn, p2m_entry, new_entry, level + 1);
+p2m_add_iommu_flags(_entry, level,
+IOMMUF_readable|IOMMUF_writable);
+rc = p2m->write_p2m_entry(p2m, gfn, p2m_entry, new_entry,
+

Re: [Xen-devel] [PATCH v4.1 4/6] xen/x86: Allow stubdom access to irq created for msi.

[Jan Beulich]

>>> On 08.02.19 at 11:17,  wrote:
> There is one code path where I haven't managed to properly extract
> possible stubdomain in use:
> pci_remove_device()
>  -> pci_cleanup_msi()
>-> msi_free_irqs()
>  -> msi_free_irq()
>-> destroy_irq()
> 
> For now I've hardcoded hardware_domain there (in msi_free_irqs). Can it happen
> when device is still assigned to some domU?

In case this question is still open: No, it can't with current code,
and provided Dom0 behaves correctly.

> @@ -368,13 +369,13 @@ static int __init hpet_assign_irq(struct 
> hpet_event_channel *ch)
>  {
>  int irq;
>  
> -if ( (irq = create_irq(NUMA_NO_NODE)) < 0 )
> +if ( (irq = create_irq(NUMA_NO_NODE, hardware_domain)) < 0 )
>  return irq;
>  
>  ch->msi.irq = irq;
>  if ( hpet_setup_msi_irq(ch) )
>  {
> -destroy_irq(irq);
> +destroy_irq(irq, hardware_domain);
>  return -EINVAL;
>  }

Why don't you take the opportunity here (and elsewhere) and properly
remove hwdom access to such internal-to-Xen IRQs? Simply pass NULL
here, and skip permission granting in this case (create_irq() already
checks for NULL anyway).

> @@ -190,19 +190,19 @@ int create_irq(nodeid_t node)
>  desc->arch.used = IRQ_UNUSED;
>  irq = ret;
>  }
> -else if ( hardware_domain )
> +else if ( dm_domain )
>  {
> -ret = irq_permit_access(hardware_domain, irq);
> +ret = irq_permit_access(dm_domain, irq);

Doesn't this imply that Dom0 has no way of cleaning up after the
guest/stubdom pair? IOW I wonder whether both dm and hwdom
should be granted access.

> @@ -2095,7 +2099,9 @@ int map_domain_pirq(
>  irq = info->arch.irq;
>  }
>  msi_desc->irq = -1;
> -msi_free_irq(msi_desc);
> +msi_free_irq(msi_desc,
> + current->domain->target == d ? current->domain
> +  : hardware_domain);

Note how ->irq gets set to -1 prior to the call (and also in at least
one other instance), which will lead to skipping of the destroy_irq()
call, and hence skipping of the permission removal. Or wait, that's
going to be taken care of in the caller as it seems. If this is also
your understanding, then please add a sentence to the description
pointing this out. The split logic isn't really helpful here (I know it
was me who wrote it, in an attempt to avoid re-writing everything
basically from scratch).

Jan



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

Re: [Xen-devel] XEN on R-CAR H3

[Julien Grall]

Hi Amit,

On 2/21/19 6:15 PM, Amit Tomer wrote:

Hi,


diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index d9836779d1..08b9cd2c44 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1805,6 +1805,8 @@ static void __init dtb_load(struct kernel_info *kinfo)
  printk("Loading dom0 DTB to 0x%"PRIpaddr"-0x%"PRIpaddr"\n",
 kinfo->dtb_paddr, kinfo->dtb_paddr + fdt_totalsize(kinfo->fdt));

+dump_p2m_lookup(kinfo->d, kinfo->dtb_paddr);
+
  left = copy_to_guest_phys_flush_dcache(kinfo->d, kinfo->dtb_paddr,
 kinfo->fdt,
 fdt_totalsize(kinfo->fdt));


Please find the logs after applying this patch:

(XEN) CPU2 will call ARM_SMCCC_ARCH_WORKAROUND_1 on exception entry
(XEN) *** LOADING DOMAIN 0 ***
(XEN) Loading Domd0 kernel from boot module @ 7a00
(XEN) Allocating 1:1 mappings totalling 512MB for dom0:
(XEN) BANK[0] 0x005000-0x007000 (512MB)
(XEN) Grant table range: 0x004800-0x004804
(XEN) Allocating PPI 16 for event channel interrupt
(XEN) Loading zImage from 7a00 to 5008-5188
(XEN) Loading dom0 DTB to 0x5800-0x58010a44
(XEN) dom0 IPA 0x5800
(XEN) P2M @ 00080c23dc90 mfn:0x73ff5e
(XEN) 0TH[0x0] = 0x00873ff5c7ff
(XEN) 1ST[0x1] = 0x00873ff3d7ff
(XEN) 2ND[0xc0] = 0x02c0580006fd


Thank you for giving a try! The p2m type for the entry is p2m_mmio_direct_c.

This confirms the finding on the other threads. I would suggest you to 
remove the reserved-memory nodes until Xen gain knowledge of reserved 
memory.


FIY, Stefano has posted a patch series yesterday aiming to solve this 
issues.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH 1/1] tools/ocaml: Dup2 /dev/null to stdin in daemonize()

[Andrew Cooper]

On 27/02/2019 10:33, Christian Lindig wrote:
> Don't close stdin in daemonize() but dup2 /dev/null instead. This avoids
> fd 0 being reused and potentially written to.
>
> Signed-off-by: Christian Lindig 

Possibly worth noting that this fixes a bug whereby /dev/xen/evtchn
reliably gets opened on fd 0.

I can fix the wording up on commit if there are no other concerns.

Reviewed-by: Andrew Cooper , and CC'ing
Juergen for 4.12

> ---
>  tools/ocaml/xenstored/stdext.ml | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/ocaml/xenstored/stdext.ml b/tools/ocaml/xenstored/stdext.ml
> index 879565c515..ffb516a0d4 100644
> --- a/tools/ocaml/xenstored/stdext.ml
> +++ b/tools/ocaml/xenstored/stdext.ml
> @@ -100,9 +100,9 @@ let daemonize () =
>  
>   begin match Unix.fork () with
>   | 0 ->
> - let nullfd = Unix.openfile "/dev/null" [ Unix.O_WRONLY 
> ] 0 in
> + let nullfd = Unix.openfile "/dev/null" [ Unix.O_RDWR] 0 
> in
>   begin try
> - Unix.close Unix.stdin;
> + Unix.dup2 nullfd Unix.stdin;
>   Unix.dup2 nullfd Unix.stdout;
>   Unix.dup2 nullfd Unix.stderr;
>   with exn -> Unix.close nullfd; raise exn


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

Re: [Xen-devel] [PATCH for-4.12 RFC] xen/console: Handle NUL character in buffer sent via CONSOLEIO_write

[Julien Grall]

(+ Juergen Gross as RM)

I forgot to CC Juergen for this.

On 2/26/19 11:03 PM, Julien Grall wrote:

After upgrading Debian to Buster, I started noticing console mangling
when using zsh. This is happenning because output sent by zsh to the
console may contain NUL character in the middle of the buffer.

Linux is sending the buffer as it is to Xen console via CONSOLEIO_write.
However, the implementation in Xen considers NUL character is used to
terminate the buffer and therefore will ignore anything after it.

The actual documentation of CONSOLEIO_write is pretty limited. From the
declaration, the hypercall takes a buffer and size. So this could lead
to think the NUL character is allowed in the middle of the buffer.

This patch updates the console API to pass the size along the buffer
down so we can remove the reliance on buffer terminating by a NUL
character.

Signed-off-by: Julien Grall 

---

This is an early RFC to start getting feedback on the issue and raise
awareness on the problem.

This patch is candidate for Xen 4.12. Without it zsh output gets mangled
when using the upcoming Debian Buster. A workaround is to add in your
.zshrc:

setopt single_line_zle

For the longer bits, it looks like zsh is now adding NUL characters in
the middle of the output sent onto the console. Below an easy way to
repro it the bug on Xen:

int main(void)
{
 write(1,
   
"\r\33[0m\0\0\0\0\0\0\0\0\33[27m\33[24m\33[j\33[32mjulien\33[31m@\33[00m\33[36mjuno2-julieng:~\33[37m>",
   75);
 write(1,
   "\33[K\33[32C\33[01;33m--juno2-julieng-13:44--\33[00m\33[37m\33[55D",
   54);
 write(1, "\33[?2004h", 8);

 return 0;
}

Without this patch, the only --juno2-julieng-13:44-- will be printed in
yellow.

This patch was tested on Arm using serial console. I am not entirely
whether the video and PV console is correct. I would appreciate help for
testing here.

TODO: Actually document CONSOLEIO_write in the public header.

---
  xen/arch/arm/early_printk.c   | 14 +-
  xen/drivers/char/console.c| 37 +++--
  xen/drivers/char/consoled.c   |  4 ++--
  xen/drivers/char/serial.c |  8 +---
  xen/drivers/char/xen_pv_console.c | 10 +-
  xen/drivers/video/lfb.c   | 14 --
  xen/drivers/video/lfb.h   |  4 ++--
  xen/drivers/video/vga.c   | 14 --
  xen/include/xen/console.h |  2 +-
  xen/include/xen/early_printk.h|  2 +-
  xen/include/xen/pv_console.h  |  4 ++--
  xen/include/xen/serial.h  |  4 ++--
  xen/include/xen/video.h   |  4 ++--
  13 files changed, 66 insertions(+), 55 deletions(-)

diff --git a/xen/arch/arm/early_printk.c b/xen/arch/arm/early_printk.c
index 97466a12b1..dd2e9fb46f 100644
--- a/xen/arch/arm/early_printk.c
+++ b/xen/arch/arm/early_printk.c
@@ -17,13 +17,17 @@
  void early_putch(char c);
  void early_flush(void);
  
-void early_puts(const char *s)

+void early_puts(const char *s, unsigned int nr)
  {
-while (*s != '\0') {
-if (*s == '\n')
+unsigned int i;
+
+for ( i = 0; i < nr; i++ )
+{
+char c = *s;
+
+if (c == '\n')
  early_putch('\r');
-early_putch(*s);
-s++;
+early_putch(c);
  }
  
  /*

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 4315588f05..cce1211a0c 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -325,9 +325,9 @@ long read_console_ring(struct xen_sysctl_readconsole *op)
  static char serial_rx_ring[SERIAL_RX_SIZE];
  static unsigned int serial_rx_cons, serial_rx_prod;
  
-static void (*serial_steal_fn)(const char *) = early_puts;

+static void (*serial_steal_fn)(const char *, unsigned int nr) = early_puts;
  
-int console_steal(int handle, void (*fn)(const char *))

+int console_steal(int handle, void (*fn)(const char *, unsigned int nr))
  {
  if ( (handle == -1) || (handle != sercon_handle) )
  return 0;
@@ -345,15 +345,15 @@ void console_giveback(int id)
  serial_steal_fn = NULL;
  }
  
-static void sercon_puts(const char *s)

+static void sercon_puts(const char *s, unsigned int nr)
  {
  if ( serial_steal_fn != NULL )
-(*serial_steal_fn)(s);
+(*serial_steal_fn)(s, nr);
  else
-serial_puts(sercon_handle, s);
+serial_puts(sercon_handle, s, nr);
  
  /* Copy all serial output into PV console */

-pv_console_puts(s);
+pv_console_puts(s, nr);
  }
  
  static void dump_console_ring_key(unsigned char key)

@@ -388,8 +388,8 @@ static void dump_console_ring_key(unsigned char key)
  }
  buf[sofar] = '\0';
  
-sercon_puts(buf);

-video_puts(buf);
+sercon_puts(buf, sofar);
+video_puts(buf, sofar);
  
  free_xenheap_pages(buf, order);

  }
@@ -527,7 +527,7 @@ static inline void xen_console_write_debug_port(const char 
*buf, size_t len)
  static long 

Re: [Xen-devel] [PATCH for-4.12 RFC] xen/console: Handle NUL character in buffer sent via CONSOLEIO_write

[Julien Grall]

Hi,

On 2/27/19 10:25 AM, Jan Beulich wrote:

On 27.02.19 at 00:03,  wrote:

After upgrading Debian to Buster, I started noticing console mangling
when using zsh. This is happenning because output sent by zsh to the
console may contain NUL character in the middle of the buffer.

Linux is sending the buffer as it is to Xen console via CONSOLEIO_write.
However, the implementation in Xen considers NUL character is used to
terminate the buffer and therefore will ignore anything after it.

The actual documentation of CONSOLEIO_write is pretty limited. From the
declaration, the hypercall takes a buffer and size. So this could lead
to think the NUL character is allowed in the middle of the buffer.

This patch updates the console API to pass the size along the buffer
down so we can remove the reliance on buffer terminating by a NUL
character.


We don't need the behavior for internal producers, so I think the change
touches way too much code. I think all you need to do is make the
hypercall handler sense null characters, and perhaps simply invoke lower
level handlers multiple times. Or replace them by something else (e.g. a
blank).


I have to disagree here. If the OS decides to pass a buffer containing 
NUL character, then we should honor it and send the NUL character to the 
serial. Otherwise you may have a different behavior when running on 
baremetal and on Xen. One case I have in mind is debugging over HVC console.


So we need to modify the console API for handling this purpose. Yes, it 
will allow the internal producers to put NUL character in it. But that's 
not a real issue by itself.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH 1/6] xen: extend XEN_DOMCTL_memory_mapping to handle cacheability

[Jan Beulich]

>>> On 27.02.19 at 00:07,  wrote:
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -571,12 +571,14 @@ struct xen_domctl_bind_pt_irq {
>  */
>  #define DPCI_ADD_MAPPING 1
>  #define DPCI_REMOVE_MAPPING  0
> +#define CACHEABILITY_DEVMEM  0 /* device memory, the default */
> +#define CACHEABILITY_MEMORY  1 /* normal memory */
>  struct xen_domctl_memory_mapping {
>  uint64_aligned_t first_gfn; /* first page (hvm guest phys page) in range 
> */
>  uint64_aligned_t first_mfn; /* first page (machine page) in range */
>  uint64_aligned_t nr_mfns;   /* number of pages in range (>0) */
>  uint32_t add_mapping;   /* add or remove mapping */
> -uint32_t padding;   /* padding for 64-bit aligned structure */
> +uint32_t cache_policy;  /* cacheability of the memory mapping */
>  };

I don't think DEVMEM and MEMORY are anywhere near descriptive
enough, nor - if we want such control anyway - flexible enough. I
think what you want is to actually specify cachability, allowing on
x86 to e.g. map frame buffers or alike WC. The attribute then
would (obviously and necessarily) be architecture specific.

In vPCI code the question then will become whether prefetchable
BARs shouldn't be mapped e.g. WT instead of UC (and whatever
the Arm equivalent is, assuming PCI support will eventually get
added to Arm64, as it seems it has been the intention for quite
some time).

Jan



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

[Xen-devel] [PATCH 1/1] tools/ocaml: Dup2 /dev/null to stdin in daemonize()

[Christian Lindig]

Don't close stdin in daemonize() but dup2 /dev/null instead. This avoids
fd 0 being reused and potentially written to.

Signed-off-by: Christian Lindig 
---
 tools/ocaml/xenstored/stdext.ml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/ocaml/xenstored/stdext.ml b/tools/ocaml/xenstored/stdext.ml
index 879565c515..ffb516a0d4 100644
--- a/tools/ocaml/xenstored/stdext.ml
+++ b/tools/ocaml/xenstored/stdext.ml
@@ -100,9 +100,9 @@ let daemonize () =
 
begin match Unix.fork () with
| 0 ->
-   let nullfd = Unix.openfile "/dev/null" [ Unix.O_WRONLY 
] 0 in
+   let nullfd = Unix.openfile "/dev/null" [ Unix.O_RDWR] 0 
in
begin try
-   Unix.close Unix.stdin;
+   Unix.dup2 nullfd Unix.stdin;
Unix.dup2 nullfd Unix.stdout;
Unix.dup2 nullfd Unix.stderr;
with exn -> Unix.close nullfd; raise exn
-- 
2.20.1


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

Re: [Xen-devel] [PATCH for-4.12 RFC] xen/console: Handle NUL character in buffer sent via CONSOLEIO_write

[Jan Beulich]

>>> On 27.02.19 at 00:03,  wrote:
> After upgrading Debian to Buster, I started noticing console mangling
> when using zsh. This is happenning because output sent by zsh to the
> console may contain NUL character in the middle of the buffer.
> 
> Linux is sending the buffer as it is to Xen console via CONSOLEIO_write.
> However, the implementation in Xen considers NUL character is used to
> terminate the buffer and therefore will ignore anything after it.
> 
> The actual documentation of CONSOLEIO_write is pretty limited. From the
> declaration, the hypercall takes a buffer and size. So this could lead
> to think the NUL character is allowed in the middle of the buffer.
> 
> This patch updates the console API to pass the size along the buffer
> down so we can remove the reliance on buffer terminating by a NUL
> character.

We don't need the behavior for internal producers, so I think the change
touches way too much code. I think all you need to do is make the
hypercall handler sense null characters, and perhaps simply invoke lower
level handlers multiple times. Or replace them by something else (e.g. a
blank).

Jan



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

Re: [Xen-devel] [RFC PATCH 0/4] Add missing default labels to switch statements

[Jan Beulich]

>>> On 27.02.19 at 10:23,  wrote:

> 
>> On 26 Feb 2019, at 18:27, Stefano Stabellini  wrote:
>> 
>> On Tue, 26 Feb 2019, Jan Beulich wrote:
 So, we'll end up having lots of macros then which is obviously
 not good. That said, I hope we can work out some common approach
 not only to this issue, but how we deal with such in general.
>>> 
>>> I guess then I have to ask for giving a complete picture of what
>>> other code uglifications are going to be proposed. If, to be MISRA-
>>> compliant, we have to turn all of our code into an unreadable
>>> mess, than I'm afraid I have to question whether we really want
>>> to go that route. We then may be better off stopping the whole
>>> exercise now, rather than after having done several initial steps
>>> already.
>> 
>> Hi Jan,
>> 
>> I don't think there is a simple answer to your point. But the best way
>> to get an idea is to give a look at MISRA-C [1]. It's less than 50GBP, I
>> am hoping Lars will be able to sort it out for you. The purpose of this
>> work is also to provide the context for the upcoming f2f discussions.
> 
> 
> I can certainly do this: the cost of buying a few copies of MISRA C standard 
> documents for a few committers is something I can approve without needing 
> advisory board approval. The documents are shipped in PDF and the license is 
> single user. To buy them I need the names and e-mail addresses of those who 
> need it. 
> 
> I recall that I read in an earlier thread that Julien and Stefano have 
> access to the document, which would leave Jan and a few members of Citrix 
> staff. Can those committers who need access raise their hands? I can then go 
> ahead and order these.

Well, you've effectively raised my hand already. To be honest I'm not
sure I want it raised: I fear to break in tears when I would get to read
that book. In any event, I'd say ...

> Having followed this thread (and the other MISRA related one from Stefano), 
> it seems to me that potentially each of these discussions is quite divisive 
> and take up a lot of discussion and emotional energy. With 143 rules and 16 
> "directives" (more like guidance) and some of the rules being mandatory (73%) 
> and some advisory (27%), but the possibility to justify deviating from the 
> rule, maybe we are approaching this wrongly. 
> 
> I have some thoughts about the approach and will follow up on this thread 
> later today or tomorrow when I had some more time to clarify my thoughts.

... don't order anything before we aren't clear whether we really want
to do this (or even any part thereof) to the code base.

Jan



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

Re: [Xen-devel] [PATCH v4] x86/nmi: correctly check MSB of P6 performance counter MSR in watchdog

[Jan Beulich]

>>> On 26.02.19 at 18:44,  wrote:
> The logic currently tries to work out if a recent overflow (that indicates
> that NMI comes from the watchdog) happened by checking MSB of performance
> counter MSR that is initially sign extended from a negative value
> that we program it to. A possibly incorrect assumption here is that
> MSB is always bit 32 while on modern hardware it's usually 47 and
> the actual bit-width is reported through CPUID. Checking bit 32 for
> overflows is usually fine since we never program it to anything
> exceeding 32-bits and NMI is handled shortly after overflow occurs.
> 
> A problematic scenario that we saw occurs on systems where SMIs taking
> significant time are possible. In that case, NMI handling is deferred to
> the point firmware exits SMI which might take enough time for the counter
> to go through bit 32 and set it to 1 again. So the logic described above
> will misread it and report an unknown NMI erroneously.
> 
> Fortunately, we can use the actual MSB, which is usually higher than the
> currently hardcoded 32, and treat this case correctly at least on modern
> hardware.
> 
> Signed-off-by: Igor Druzhinin 

Reviewed-by: Jan Beulich 
albeit ...

> @@ -323,6 +326,15 @@ static void setup_p6_watchdog(unsigned counter)
>  unsigned int evntsel;
>  
>  nmi_perfctr_msr = MSR_P6_PERFCTR(0);
> +if ( !nmi_p6_event_width )
> + nmi_p6_event_width = (current_cpu_data.cpuid_level >= 0xa) ?
> + MASK_EXTR(cpuid_eax(0xa), P6_EVENT_WIDTH_MASK) :
> + P6_EVENT_WIDTH_MIN;
> +if ( !nmi_p6_event_width )
> +nmi_p6_event_width = P6_EVENT_WIDTH_MIN;

... I think this would now better be

if ( !nmi_p6_event_width && current_cpu_data.cpuid_level >= 0xa )
nmi_p6_event_width = MASK_EXTR(cpuid_eax(0xa), P6_EVENT_WIDTH_MASK);
if ( !nmi_p6_event_width )
nmi_p6_event_width = P6_EVENT_WIDTH_MIN;

Re-writing of which also mad me notice a hard tab in there. I'd be
fine making the adjustment while committing, as long as you agree.

Btw, considering the combination of subject tag and Cc list I take it
that you don't intend this to go into 4.12? I ask because generally
I'd consider this a backporting candidate.

Jan



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

Re: [Xen-devel] [PATCH v10 3/6] xen/arm: use DEFINE_SYMBOL as required

[Jan Beulich]

>>> On 26.02.19 at 20:20,  wrote:
> On Tue, 26 Feb 2019, Ian Jackson wrote:
>> Having written all that down (what a palaver), we can see that your
>> cast, above, is a breach of the rules.  Instead you can just pass the
>> struct abstract_alt_instr*, and all is well.  Then you don't need a
>> comment at the use site, either, since you are doing things which are
>> entirely supported and explained.
> 
> The in-code comment is great, and I have added it to the right patch.
> 
> Let me get a clarification on the rest of the suggestion: I would
> have to change the type of region->end to const struct
> abstract_alt_instr* (see xen/arch/arm/alternative.c).
> 
> If I do that, I get a compilation failure at:
> 
> alternative.c:245:16: error: initialization from incompatible pointer type 
> [-Werror=incompatible-pointer-types]
>  .end = end,
> 
> because apply_alternatives currently expects two struct alt_instr* as
> parameters. I cannot change the type of the second parameter of
> apply_alternatives, because actually it might not be a linker symbol, it
> might not be a struct abstract_alt_instr*. So I would still have to cast
> to struct abstract_alt_instr* somewhere?

I don't think so, no. In livepatch.c we have

end = sec->load_addr + sec->sec->sh_size;

which (a) is effectively a linker defined symbol, just that we don't
obtain it through a linker script and it also has no name associated
with it and (b) will be fine without any casts thanks to load_addr
being of type void * (i.e. the type of "end" can freely change).

>> > -memset(p, 0, __per_cpu_data_end - __per_cpu_start);
>> > -__per_cpu_offset[cpu] = p - __per_cpu_start;
>> > +memset(p, 0, per_cpu_diff(__per_cpu_start, __per_cpu_data_end));
>> > +__per_cpu_offset[cpu] = (uintptr_t)p - (uintptr_t)__per_cpu_start;
>> 
>> If per_cpu_diff(__per_cpu_start, __per_cpu_data_end) gives the right
>> value for memset, isn't it the right value for offset[cpu] too ?
>> Ie I don't know why you are using uintptr_t here.
> 
> I am using uintptr_t because p is not a abstract_per_cpu type. I could
> do:
> 
>   __per_cpu_offset[cpu] = per_cpu_diff(__per_cpu_start,
>  (struct abstract_per_cpu *)p);
> 
> I didn't think it was better though. What do you think?

Did you try changing p's type? That said, the calculation isn't going
to be universally correct (in MISRA's sense), no matter what you do:
We _deliberately_ subtract two entities here which are _guaranteed_
not to point to the same object (or one past an array's last element).

>> > @@ -37,7 +38,7 @@ static void _free_percpu_area(struct rcu_head *head)
>> >  {
>> >  struct free_info *info = container_of(head, struct free_info, rcu);
>> >  unsigned int cpu = info->cpu;
>> > -char *p = __per_cpu_start + __per_cpu_offset[cpu];
>> > +char *p = (char *)__per_cpu_start + __per_cpu_offset[cpu];
>> 
>> I also don't know why you are casting to char* here if you didn't need
>> to do so before.
> 
> That is because __per_cpu_start is now const, it wasn't before.

That's why I'm advocating that free()-style functions should take
pointers to const. A patch to this effect wasn't liked, though.

Jan



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

Re: [Xen-devel] [PATCH v10 2/6] xen: introduce DEFINE_SYMBOL

[Jan Beulich]

>>> On 26.02.19 at 19:43,  wrote:
> On Tue, 26 Feb 2019, Ian Jackson wrote:
>> Jan Beulich writes ("Re: [PATCH v10 2/6] xen: introduce DEFINE_SYMBOL"):
>> > On 26.02.19 at 17:46,  wrote:
>> > > I am not aware of a standard C type which could be used instead of
>> > > this struct.  But I think you can use the `packed' attribute to get
>> > > the right behaviour.  The GCC manual says:
>> > > 
>> > >  | Alignment can be decreased by specifying the 'packed' attribute.
>> > >  | See below.
>> ...
>> > Until I've looked at this (again) now, I wasn't even aware that
>> > one can combine packed and aligned attributes in a sensible
>> > way. May I suggest that, because of this being a theoretical
>> > issue only at this point, we limit ourselves to the build time
>> > assertion you suggest?
>> 
>> I am not suggesting combining `packed' and `aligned'.  I am suggesting
>> only `packed' (but based on text which is in the manual section for
>> `aligned').  But I am happy with a build-time assertion if you don't
>> want to add `packed'.  That is just as safe.
> 
> Could you please provide a rough example of the build-time assertion you
> are thinking about? I am happy to add it.

BUILD_BUG_ON(alignof(*s1) != alignof(*s2));

Jan



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

Re: [Xen-devel] [PATCH v10 4/6] xen/x86: use DEFINE_SYMBOL as required

[Jan Beulich]

>>> On 26.02.19 at 20:23,  wrote:
> On Tue, 26 Feb 2019, Ian Jackson wrote:
>> Stefano Stabellini writes ("[PATCH v10 4/6] xen/x86: use DEFINE_SYMBOL as 
>> required"):
>> > Use SYMBOLS_SUBTRACT and SYMBOLS_COMPARE in cases of comparisons and
>> > subtractions of:
>> ...
>> > Use explicit casts to uintptr_t when it is not possible to use the
>> > provided static inline functions.
>> 
>> Why is it not possible ?  You write:
>> 
>> > +/*
>> > + * Cannot use DEFINE_SYMBOL because of the way they are passed to
>> > + * apply_alternatives.
>> > + */
>> >  extern struct alt_instr __alt_instructions[], __alt_instructions_end[];
>> 
>> But I don't know why you can't pass a `struct abstract_alt_instr*' to
>> apply_alternatives.
>> 
>> IMO it should be strictly forbidden to ever write this formulation, as
>> you have above.  See my proposed rule comment for DEFINE_SYMBOL.
>> 
>> Even if you can't use the macros at some particular calculation site,
>> you should still ensure that ..._end has a different type, to make
>> sure that no unsafe uses escape.
> 
> Unfortunately __apply_alternatives is called from
> __apply_alternatives_multi_stop, where it would be fine to use struct
> abstract_alt_instr*, and also from apply_alternatives which in a
> xen/common interface called from xen/common/livepatch.c and doesn't work
> with linker symbols AFAICT.

As Ian says, it's only a matter of correctly defining the type of "end" at
the call site.

Jan



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

Re: [Xen-devel] [RFC PATCH 0/4] Add missing default labels to switch statements

[Lars Kurth]

> On 26 Feb 2019, at 18:27, Stefano Stabellini  wrote:
> 
> On Tue, 26 Feb 2019, Jan Beulich wrote:
>>> So, we'll end up having lots of macros then which is obviously
>>> not good. That said, I hope we can work out some common approach
>>> not only to this issue, but how we deal with such in general.
>> 
>> I guess then I have to ask for giving a complete picture of what
>> other code uglifications are going to be proposed. If, to be MISRA-
>> compliant, we have to turn all of our code into an unreadable
>> mess, than I'm afraid I have to question whether we really want
>> to go that route. We then may be better off stopping the whole
>> exercise now, rather than after having done several initial steps
>> already.
> 
> Hi Jan,
> 
> I don't think there is a simple answer to your point. But the best way
> to get an idea is to give a look at MISRA-C [1]. It's less than 50GBP, I
> am hoping Lars will be able to sort it out for you. The purpose of this
> work is also to provide the context for the upcoming f2f discussions.


I can certainly do this: the cost of buying a few copies of MISRA C standard 
documents for a few committers is something I can approve without needing 
advisory board approval. The documents are shipped in PDF and the license is 
single user. To buy them I need the names and e-mail addresses of those who 
need it. 

I recall that I read in an earlier thread that Julien and Stefano have access 
to the document, which would leave Jan and a few members of Citrix staff. Can 
those committers who need access raise their hands? I can then go ahead and 
order these.

Having followed this thread (and the other MISRA related one from Stefano), it 
seems to me that potentially each of these discussions is quite divisive and 
take up a lot of discussion and emotional energy. With 143 rules and 16 
"directives" (more like guidance) and some of the rules being mandatory (73%) 
and some advisory (27%), but the possibility to justify deviating from the 
rule, maybe we are approaching this wrongly. 

I have some thoughts about the approach and will follow up on this thread later 
today or tomorrow when I had some more time to clarify my thoughts.

Regards
Lars  


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

Re: [Xen-devel] [PATCH 0/3] mwait support for AMD processors

[Jan Beulich]

>>> On 26.02.19 at 17:54,  wrote:
> On 2/26/19 10:37 AM, Jan Beulich wrote:
> On 26.02.19 at 17:25,  wrote:
>>> Correct me if I'm wrong, but the Xen's acpi-idle implementation is
>>> dependent on dom0 using a AML interpreter and then giving that data back
>>> to Xen.  I've heard that this doesn't always work correctly on PV dom0s
>>> and doesn't work at all on PVH dom0s.
>> 
>> For C2 and deeper (using entering methods other than HLT) - yes.
>> The use of HLT is the default with the assumption that this will put
>> the system in C1 (i.e. with a pretty low wakeup latency); see
>> default_idle(), cpuidle_init_cpu(), and acpi_idle_do_entry().
> 
> Well, assuming C2 is enabled (which I was assume is the default case), 
> HLT roughly puts the processor in C2 rather than C1.  On my test system, 
> the debug console output for the cx tables only output HLT for C1 (which 
> is wrong).
> 
> Rather than depending on dom0, which is shaky, and not having an AML 
> interpreter, it seems the best solution is to hardcode the values in 
> like Intel does.  If Xen had an AML interpreter, I'd agree doing things 
> differently (reading in the ACPI tables) would be best.  But given the 
> resources Xen has at the moment, this seems like the safest solution and 
> is better than using HLT (which is C2 assuming it's enabled) as the 
> default idle method like Xen is using now.
> 
> It comes down to sometimes (when C2 is diabled in BIOS) using C1 
> thinking it's C2, or without the patches in the common case using C2 
> thinking it's C1.

So in one of our idle routines, how would one go about entering
C1 or C2 depending on wakeup latency requirements? I'm having a
hard time seeing how HLT can be used for both (without a reboot
cycle and a BIOS option change in between). Yet if there's only
one state that can be entered, then it's merely cosmetic whether
it gets called C1 or C2 in the debug output.

Anyway - I guess we need to continue this discussion (if necessary)
once I got around to actually look at the patches.

Jan



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

Re: [Xen-devel] [PATCH v10 6/6] xen: use DEFINE_SYMBOL as required

[Jan Beulich]

>>> On 26.02.19 at 22:22,  wrote:
> On Tue, 26 Feb 2019, Jan Beulich wrote:
>> >>> On 25.02.19 at 21:50,  wrote:
>> > @@ -210,7 +210,8 @@ static int __apply_alternatives_multi_stop(void 
>> > *unused)
>> >  region.begin = __alt_instructions;
>> >  region.end = (struct alt_instr *)__alt_instructions_end;
>> >  
>> > -ret = __apply_alternatives(, xenmap - (void *)_start);
>> > +ret = __apply_alternatives(, (uintptr_t)xenmap -
>> > +(uintptr_t)_start);
>> 
>> Undesirable (but in this case maybe indeed unavoidable) casting
>> instead. I don't think this belongs in a patch with the given title
>> though.
> 
> It's in this patch because this is the patch dealing with _start and
> _end. Let me know how would you like the patches to be split.

Well, I can see the general possible need for additional changes
due to the type adjustments. I don't see though why the original
code in this example would break with the other adjustments done
here. Things need to build and work after each patch, but changes
not strictly needed and not related to the subject of a patch would
better be split out (in this case into the [or another] Arm-specific
patch).

>> > @@ -78,7 +78,19 @@ void arch_livepatch_revert(const struct livepatch_func 
>> > *func)
>> >  uint32_t *new_ptr;
>> >  unsigned int len;
>> >  
>> > -new_ptr = func->old_addr - (void *)_start + vmap_of_xen_text;
>> > +/*
>> > + * We need to calculate the offset of the address from _start, and
>> > + * apply that to our own map, to find where we have this mapped.
>> > + * Doing these kind of games directly with pointers is contrary to
>> > + * the C rules for what pointers may be compared and computed.  So
>> > + * we do the offset calculation with integers, which is always
>> > + * legal.  The subsequent addition of the offset to the
>> > + * vmap_of_xen_text pointer is legal because the computed pointer is
>> > + * indeed a valid part of the object referred to by vmap_of_xen_text
>> > + * - namely, the byte array of our mapping of the Xen text.
>> > + */
>> > +new_ptr = ((uintptr_t)func->old_addr, - (uintptr_t)_start) +
>> > +  vmap_of_xen_text;
>> 
>> You not using the intended helper inlines has allowed for a bug to
>> slip in that the compiler can't even help notice, due to - being both
>> a valid unary and a valid binary operator.
> 
> Well spotted! I'll fix the bug. I would also be happy to use the helper
> inlines, but we discussed not to use them in cases like this, with three
> operators. Even if I wanted to use them, none of the inline helpers fit
> this case. Or do you suggest:
> 
>   new_ptr = xen_diff(_start, (struct abstract_xen *)func->old_addr) +
> vmap_of_xen_text;
> 
> Is that what you are asking?

No matter what, it looks like you're wanting (and perhaps needing) to
stick to some form of cast here. But that's what you're specifically
trying to get away from, aren't you? What is MISRA's position on
casts from void * to a type? This is not a generally "safe" operation
after all, because the casted to type could be out of sync with the
object the pointer points at.

Note that struct livepatch_func only happens to live in the public
interface, but the type of old_addr ought to be freely changeable as
long as it remains a pointer. Did you check whether changing it would
help avoid all casting?

Jan



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

Re: [Xen-devel] [PATCH v10 5/6] xen/common: use DEFINE_SYMBOL as required

[Jan Beulich]

>>> On 26.02.19 at 22:14,  wrote:
> On Tue, 26 Feb 2019, Jan Beulich wrote:
>> >>> On 25.02.19 at 21:50,  wrote:
>> > --- a/xen/common/lib.c
>> > +++ b/xen/common/lib.c
>> > @@ -492,12 +492,15 @@ unsigned long long parse_size_and_unit(const char 
>> > *s, 
>> > const char **ps)
>> >  }
>> >  
>> >  typedef void (*ctor_func_t)(void);
>> > -extern const ctor_func_t __ctors_start[], __ctors_end[];
>> > +DEFINE_SYMBOL(ctor_func_t, ctor_func, __ctors_start, __ctors_end);
>> 
>> At the example of this, there's too much redundancy here for my
>> taste. At least the _start and _end suffixes should be made
>> consistent across the code base (maybe except for the pseudo-
>> standard symbols like _etext and _edata). I'd also prefer if what
>> is now the first parameter would simply become ##_t.
>> There's nothing wrong with adding a few typedefs for this to work
>> in the common case.
> 
> I understand your point but I would prefer to avoid changing the
> existing types or names. Instead, I could add a wrapper around
> DEFINE_SYMBOL or DECLARE_BOUNDS as you suggested, see my other reply
> https://marc.info/?l=xen-devel=155120735032147.
> 
> However, this example doesn't actually follow the regular pattern
> unfortunately (__ctors_start != ctors_func_start). I would like to avoid
> making all names/types follow the regular pattern as part of this
> series. I could do as a clean-up afterwards.

Personally I think the bringing in line with the intended common
pattern should be a prereq patch (or several of them if need be)
in this series.

>> > @@ -147,14 +148,14 @@ static int __init xen_build_init(void)
>> >  int rc;
>> >  
>> >  /* --build-id invoked with wrong parameters. */
>> > -if ( __note_gnu_build_id_end <= [0] )
>> > +if ( !elf_note_lt([0], __note_gnu_build_id_end) )
>> >  return -ENODATA;
>> >  
>> >  /* Check for full Note header. */
>> > -if ( [1] >= __note_gnu_build_id_end )
>> > +if ( !elf_note_lt([1], __note_gnu_build_id_end) )
>> >  return -ENODATA;
>> >  
>> > -sz = (void *)__note_gnu_build_id_end - (void *)n;
>> > +sz = (uintptr_t)__note_gnu_build_id_end - (uintptr_t)n;
>> 
>> This is another example of undue open coding. As also said on
>> the call we've had, it may be helpful to have a second diff
>> function for this, producing the byte difference instead of the
>> element one. In fact I did suggest to make the latter use the
>> former, such that the casting was truly limited to as few places
>> as possible.
> 
> I considered adding a second function, but this is the only instance we
> have today, so I decided to skip it. I am OK with adding the separate
> function though, let me know.

Well, as per the discussion also with Ian on patch 2, there's
independent benefit of having that extra function. So yes, I
continue to think it should be introduced.

Jan



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

Re: [Xen-devel] [PATCH v10 2/6] xen: introduce DEFINE_SYMBOL

[Jan Beulich]

>>> On 26.02.19 at 19:54,  wrote:
> On Tue, 26 Feb 2019, Jan Beulich wrote:
>> >>> On 25.02.19 at 21:50,  wrote:
>> > --- a/xen/include/xen/compiler.h
>> > +++ b/xen/include/xen/compiler.h
>> > @@ -99,6 +99,38 @@
>> >  __asm__ ("" : "=r"(__ptr) : "0"(ptr));  \
>> >  (typeof(ptr)) (__ptr + (off)); })
>> >  
>> > +
>> > +/*
>> > + * Declare start and end array variables in C corresponding to existing
>> > + * linker symbols.
>> 
>> You validly say "declare" here, so why ...
>> 
>> > + * Two static inline functions are declared to do comparisons and
>> > + * subtractions between these variables.
>> > + *
>> > + * The end variable is declared with a different type to make sure that
>> > + * the static inline functions cannot be misused.
>> > + */
>> > +#define DEFINE_SYMBOL(type, name, start_name, end_name)   
>> >  
>\
>> 
>> ... do you use DEFINE here?
>> 
>> How about DECLARE_ARRAY_BOUNDS(tag, name) using
>> tag ## _t as type, tag ## _lt etc as function names, and
>> name ## _start / name ## _end as start / end symbols. To
>> accommodate things like _etext, the above could in fact expand
>> to DECLARE_BOUNDS(tag, name ## _start, name ## _end)
>> allowing this second macro then to also be used like
>> DECLARE_BOUNDS(text, _stext, _etext).
> 
> I am fine with renaming the macro. It is also fine to provide a wrapper
> macro which automatically sets the most common variable names.
> 
> However, in your example both DECLARE_BOUNDS and DECLARE_ARRAY_BOUNDS
> end up assuming that the type is tag ## _t. Currently many of our
> variable types don't follow this naming pattern (struct alt_instr,
> struct device_desc, struct acpi_device_desc, just to name the first
> three I found). I don't think it is a good idea to introduce even more
> renaming as part of this series.

I didn't suggest renaming anything. Instead I've suggested to add
typedef-s as needed.

Jan



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

Re: [Xen-devel] [PATCH v10 2/6] xen: introduce DEFINE_SYMBOL

[Jan Beulich]

>>> On 26.02.19 at 18:32,  wrote:
> Jan Beulich writes ("Re: [PATCH v10 2/6] xen: introduce DEFINE_SYMBOL"):
>> On 26.02.19 at 17:46,  wrote:
>> > I am not aware of a standard C type which could be used instead of
>> > this struct.  But I think you can use the `packed' attribute to get
>> > the right behaviour.  The GCC manual says:
>> > 
>> >  | Alignment can be decreased by specifying the 'packed' attribute.
>> >  | See below.
> ...
>> Until I've looked at this (again) now, I wasn't even aware that
>> one can combine packed and aligned attributes in a sensible
>> way. May I suggest that, because of this being a theoretical
>> issue only at this point, we limit ourselves to the build time
>> assertion you suggest?
> 
> I am not suggesting combining `packed' and `aligned'.  I am suggesting
> only `packed' (but based on text which is in the manual section for
> `aligned').  But I am happy with a build-time assertion if you don't
> want to add `packed'.  That is just as safe.

But "packed" alone reduces alignment to the minimum, i.e. one byte.
That would immediately break the build time assertion you've suggested
to add.

Jan



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