Re: [Xen-devel] [BUG] win2008 guest cannot get ip through sriov

[Xu, Quan]

On June 02, 2016 8:09 PM, Jan Beulich  wrote:
> Attached both a hypervisor and a qemu patch. Their plus
> debug key M and i output is what I'd like to start with.

Applied these two patches, the attach files are logs.

-vf PT is working for SLES 12. The logs:
   qemu-dm-sles.log -- qemu log, vf PT for sles 12.
   xen-sles.log -- xen log, vf PT for sles 12.

-vf PT is not working for win2008: the logs:
   qemu-dm-win2k8.log -- qemu log, vf PT for win2008.
   xen-win2k8.log -- xen log, vf PT for win2008.


btw, when I updated the NIC driver for win2008,  actually, it is working.  
Xudong, could you help me verify it again?

((
xen commit: bbfd2d6ccb31a3aeea49c8f9c7884792ddc26e3b
NIC: Intel Corporation I350 Gigabit Network Connection
 new win2k8 NIC driver 
,https://downloadcenter.intel.com/download/18720/Network-Adapter-Driver-for-Windows-Server-2008-Final-Release?product=59679
))

qemu-dm-TestDom.log -- qemu log, pf PT for win2008.


... taken together, IMO, from qemu-dm-win2k8.log(also I have read xen/QEMU 
code), the win2k8 vf nic driver is not loading. (any idea, feel free to let me 
know. I will help you try as much as I can..).

Thoughts?

Quan


qemu-dm-sles.log
Description: qemu-dm-sles.log


qemu-dm-TestDom.log
Description: qemu-dm-TestDom.log


qemu-dm-win2k8.log
Description: qemu-dm-win2k8.log


xen-sles.log
Description: xen-sles.log


xen-win2k8.log
Description: xen-win2k8.log
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] Current PVH/HVMlite work and planning (was :Re: Discussion about virtual iommu support for Xen guest)

[Tian, Kevin]

> From: Stefano Stabellini
> Sent: Saturday, June 04, 2016 12:57 AM
> 
> > > >
> > > > How stable is the HVMLite today? Is it already in production usage?
> > > >
> > > > Wonder whether you have some detail thought how full PCI root complex
> > > > emulation will be done in Xen (including how to interact with Qemu)...
> > >
> > > I haven't looked into much detail regarding all this, since as I said, 
> > > it's
> > > still a little bit far away in the PVH/HVMlite roadmap, we have more
> > > pressing issues to solve before getting to the point of implementing
> > > PCI-passthrough. I expect Xen is going to intercept all PCI accesses and 
> > > is
> > > then going to forward them to the ioreq servers that have been registered
> > > for that specific config space, but this of course needs much more thought
> > > and a proper design document.
> > >
> > > > As I just wrote in another mail, if we just hit for HVM first, will it 
> > > > work if
> > > > we implement vIOMMU in Xen but still relies on Qemu root complex to
> > > > report to the guest?
> > >
> > > This seems quite inefficient IMHO (but I don't know that much about all 
> > > this
> > > vIOMMU stuff). If you implement vIOMMU inside of Xen, but the PCI root
> > > complex is inside of Qemu aren't you going to perform quite a lot of jumps
> > > between Xen and QEMU just to access the vIOMMU?
> > >
> > > I expect something like:
> > >
> > > Xen traps PCI access -> QEMU -> Xen vIOMMU implementation
> > >
> >
> > I hope the role of Qemu is just to report vIOMMU related information, such
> > as DMAR, etc. so guest can enumerate the presence of vIOMMU, while
> > the actual emulation is done by vIOMMU in hypervisor w/o going through
> > Qemu.
> >
> > However just realized even for above purpose, there's still some interaction
> > required between Qemu and Xen vIOMMU, e.g. register base of vIOMMU and
> > devices behind vIOMMU are reported thru ACPI DRHD which means Xen vIOMMU
> > needs to know the configuration in Qemu which might be dirty to define such
> > interfaces between Qemu and hypervisor. :/
> 
> PCI accesses don't need to be particularly fast, they should not be on
> the hot path.
> 
> How bad this interface between QEMU and vIOMMU in Xen would look like?
> Can we make a short list of basic operations that we would need to
> support to get a clearer idea?

Below is a quick thought of basic operations between vIOMMU and a PCI 
root-complex, if they are not putting together:
(come from VT-d spec, section 8, "BIOS Consideration")

1) vIOMMU reports its presence including capabilities to root-complex:
- interrupt remapping, ATS, etc.
- type of devices (virtual, PV, passthrough)... (???TBD)

2) root-complex notifies vIOMMU about:
- base of vIOMMU registers (DRHD)
- devices attached to this vIOMMU (DRHD)
* dynamic update due to hotplug or PCI resource rebalancing

3) Additionally as I mentioned in another thread, Qemu need to query
vIOMMU whether a virtual DMA should be blocked, if vIOMMU for virtual
devices are also put in Xen

Other BIOS structures (ATS, RMRR, hotplug, etc.) are optional so I haven't
thought carefully now. They may require additional interactions if required.

Thanks
Kevin
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] Discussion about virtual iommu support for Xen guest

[Tian, Kevin]

> From: Stefano Stabellini
> Sent: Saturday, June 04, 2016 1:15 AM
> 
> On Fri, 3 Jun 2016, Andrew Cooper wrote:
> > On 03/06/16 12:17, Tian, Kevin wrote:
> > >> Very sorry for the delay.
> > >>
> > >> There are multiple interacting issues here.  On the one side, it would
> > >> be useful if we could have a central point of coordination on
> > >> PVH/HVMLite work.  Roger - as the person who last did HVMLite work,
> > >> would you mind organising that?
> > >>
> > >> For the qemu/xen interaction, the current state is woeful and a tangled
> > >> mess.  I wish to ensure that we don't make any development decisions
> > >> which makes the situation worse.
> > >>
> > >> In your case, the two motivations are quite different I would recommend
> > >> dealing with them independently.
> > >>
> > >> IIRC, the issue with more than 255 cpus and interrupt remapping is that
> > >> you can only use x2apic mode with more than 255 cpus, and IOAPIC RTEs
> > >> can't be programmed to generate x2apic interrupts?  In principle, if you
> > >> don't have an IOAPIC, are there any other issues to be considered?  What
> > >> happens if you configure the LAPICs in x2apic mode, but have the IOAPIC
> > >> deliver xapic interrupts?
> > > The key is the APIC ID. There is no modification to existing PCI MSI and
> > > IOAPIC with the introduction of x2apic. PCI MSI/IOAPIC can only send
> > > interrupt message containing 8bit APIC ID, which cannot address >255
> > > cpus. Interrupt remapping supports 32bit APIC ID so it's necessary to
> > > enable >255 cpus with x2apic mode.
> >
> > Thanks for clarifying.
> >
> > >
> > > If LAPIC is in x2apic while interrupt remapping is disabled, IOAPIC cannot
> > > deliver interrupts to all cpus in the system if #cpu > 255.
> >
> > Ok.  So not ideal (and we certainly want to address it), but this isn't
> > a complete show stopper for a guest.
> >
> > >> On the other side of things, what is IGD passthrough going to look like
> > >> in Skylake?  Is there any device-model interaction required (i.e. the
> > >> opregion), or will it work as a completely standalone device?  What are
> > >> your plans with the interaction of virtual graphics and shared virtual
> > >> memory?
> > >>
> > > The plan is to use a so-called universal pass-through driver in the guest
> > > which only accesses standard PCI resource (w/o opregion, PCH/MCH, etc.)
> >
> > This is fantastic news.
> >
> > >
> > > 
> > > Here is a brief of potential usages relying on vIOMMU:
> > >
> > > a) enable >255 vcpus on Xeon Phi, as the initial purpose of this thread.
> > > It requires interrupt remapping capability present on vIOMMU;
> > >
> > > b) support guest SVM (Shared Virtual Memory), which relies on the
> > > 1st level translation table capability (GVA->GPA) on vIOMMU. pIOMMU
> > > needs to enable both 1st level and 2nd level translation in nested
> > > mode (GVA->GPA->HPA) for passthrough device. IGD passthrough is
> > > the main usage today (to support OpenCL 2.0 SVM feature). In the
> > > future SVM might be used by other I/O devices too;
> > >
> > > c) support VFIO-based user space driver (e.g. DPDK) in the guest,
> > > which relies on the 2nd level translation capability (IOVA->GPA) on
> > > vIOMMU. pIOMMU 2nd level becomes a shadowing structure of
> > > vIOMMU 2nd level by replacing GPA with HPA (becomes IOVA->HPA);
> >
> > All of these look like interesting things to do.  I know there is a lot
> > of interest for b).
> >
> > As a quick aside, does Xen currently boot on a Phi?  Last time I looked
> > at the Phi manual, I would expect Xen to crash on boot because of MCXSR
> > differences from more-common x86 hardware.

Tianyu can correct me for the detail info. Xen can boot on Xeon Phi. However
we need a hacky patch in guest Linux kernel to disable dependency check
around interrupt remapping. Otherwise guest kernel boot will fail.

Now we're suffering from some performance issue. When the analysis is
ongoing, could you elaborate the limitation you see with 64vcpu guest? It
would be helpful whether we are hunting the same problem or not...

> >
> > >
> > > 
> > > And below is my thought viability of implementing vIOMMU in Qemu:
> > >
> > > a) enable >255 vcpus:
> > >
> > >   o Enable Q35 in Qemu-Xen;
> > >   o Add interrupt remapping in Qemu vIOMMU;
> > >   o Virtual interrupt injection in hypervisor needs to know virtual
> > > interrupt remapping (IR) structure, since IR is behind vIOAPIC/vMSI,
> > > which requires new hypervisor interfaces as Andrew pointed out:
> > >   * either for hypervisor to query IR from Qemu which is not
> > > good;
> > >   * or for Qemu to register IR info to hypervisor which means
> > > partial IR knowledge implemented in hypervisor (then why not putting
> > > whole IR emulation in Xen?)
> > >
> > > b) support SVM
> > >
> > >   o Enable Q35 in Qemu-Xen;
> > >   o Add 1st level translation capability in Qemu vIOMMU;
> > >   o VT-d context entry points to guest 1st level translation table
> > 

[Xen-devel] [RFC v2] xen/arm: build: add missed dependency for head.S

[Wei Chen]

In current Xen build rules, the build system will only check the
dependencies in current folder and obj-y generated dependencies
in other folder.

But Makefile may add some objects to ALL_OBJS. These objects may
be not in the same folder as Makefile. Use arch/arm/Makefile as
an example:
ALL_OBJS := $(TARGET_SUBARCH)/head.o

The head.o is not in the same folder as Makefile and is not listed
in obj-y. So, when we update the header files that had been included
in head.S. The build system would not check the dependency of head.S.
The head.S would not be re-compiled.

In this patch, we will get objects added by Makefile and add their
dependencies to check list.

Signed-off-by: Wei Chen 
---
v1 -> v2:
- Use a generic method instead of changing Makefiles
---
 xen/Rules.mk | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index 961d533..fb3087c 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -95,12 +95,21 @@ include $(BASEDIR)/arch/$(TARGET_ARCH)/Rules.mk
 include Makefile
 
 DEPS = .*.d
+
+# In obj-y and ALL_OBJS, there may have some objects are not in the
+# same folder as Makefile, so we have to use gendep to generate
+# dependencies for them.
+DEP_OBJS = $(obj-y)
+
+# Extract objects from ALL_OBJS added by Makefile.
+DEP_OBJS += $(filter-out %built_in.o,$(ALL_OBJS))
+
 define gendep
 ifneq ($(1),$(subst /,:,$(1)))
 DEPS += $(dir $(1)).$(notdir $(1)).d
 endif
 endef
-$(foreach o,$(filter-out %/,$(obj-y)),$(eval $(call gendep,$(o
+$(foreach o,$(filter-out %/,$(DEP_OBJS)),$(eval $(call gendep,$(o
 
 # Ensure each subdirectory has exactly one trailing slash.
 subdir-n := $(patsubst %,%/,$(patsubst %/,%,$(subdir-n) $(subdir-)))
-- 
2.7.4


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

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qemuu-ovmf-amd64 17 guest-start/debianhvm.repeat fail REGR. 
vs. 94748
 test-amd64-amd64-xl-qemuu-ovmf-amd64 17 guest-start/debianhvm.repeat fail 
REGR. vs. 94748

version targeted for testing:
 ovmf 0d0c245dfb147956cb597582bc481579ceb612c0
baseline version:
 ovmf dc99315b8732b6e3032d01319d3f534d440b43d0

Last test of basis94748  2016-05-24 22:43:25 Z   13 days
Failing since 94750  2016-05-25 03:43:08 Z   12 days   32 attempts
Testing same since95318  2016-06-06 09:59:18 Z0 days2 attempts


People who touched revisions under test:
  Cohen, Eugene 
  Dandan Bi 
  Darbin Reyes 
  Eugene Cohen 
  Fu Siyuan 
  Fu, Siyuan 
  Gary Li 
  Gary Lin 
  Giri P Mudusuru 
  Hao Wu 
  Jeff Fan 
  Jiaxin Wu 
  Jiewen Yao 
  Katie Dellaquila 
  Laszlo Ersek 
  Liming Gao 
  Marvin H?user 
  Marvin Haeuser 
  Maurice Ma 
  Michael Zimmermann 
  Star Zeng 
  Thomas Palmer 
  Yonghong Zhu 

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



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

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

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

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


Not pushing.

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

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

[Xen-devel] [qemu-mainline test] 95329: regressions - trouble: broken/fail/pass

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-xl-multivcpu  3 host-install(3)broken REGR. vs. 94856
 test-amd64-i386-freebsd10-amd64 10 guest-startfail REGR. vs. 94856
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 15 guest-localmigrate/x10 fail REGR. 
vs. 94856
 test-amd64-i386-xl-qemuu-winxpsp3 15 guest-localmigrate/x10 fail REGR. vs. 
94856

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-qemuu-win7-amd64 13 guest-localmigrate fail REGR. vs. 94856
 test-amd64-i386-xl-qemuu-win7-amd64 13 guest-localmigrate fail REGR. vs. 94856
 test-amd64-amd64-xl-qemuu-winxpsp3 13 guest-localmigrate  fail REGR. vs. 94856
 test-amd64-amd64-xl-rtds  9 debian-install   fail   like 94856

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass
 test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass

version targeted for testing:
 qemuue854d0cf7847e70f5ed5dad5820fc1bbeda6f29e
baseline version:
 qemuud6550e9ed2e1a60d889dfb721de00d9a4e3bafbe

Last test of basis94856  2016-05-27 20:14:49 Z   10 days
Failing since 94983  2016-05-31 09:40:12 Z6 days   12 attempts
Testing same since95329  2016-06-06 13:44:46 Z0 days1 attempts


People who touched revisions under test:
  Alex Bennée 
  Alexander Graf 
  Anthony PERARD 
  Benjamin Herrenschmidt 
  Bharata B Rao 
  Chen Fan 
  Cole Robinson 
  Cédric Le Goater 
  David Gibson 
  Denis V. Lunev 
  Dmitry Fleytman 
  Dmitry Fleytman 
  Drew Jones 
  Eduardo Habkost 
  Emilio G. Cota 
  Eric Blake 
  Fam Zheng 
  Gerd Hoffmann 
  Gu Zheng 
  Jason Wang 
  Jean-Christophe Dubois 
  Leonid Bloch 
  Li Zhijian 
  Mark Cave-Ayland 

Re: [Xen-devel] questions of vm save/restore on arm64

[Chenxiao Zhao]

On 6/6/2016 7:58 PM, Stefano Stabellini wrote:

On Fri, 3 Jun 2016, Chenxiao Zhao wrote:

On 6/3/2016 4:02 AM, Julien Grall wrote:

Hello,

First thing, the time in the mail headers seems to be wrong. Maybe
because of a wrong timezone?

I got: 04/06/16 02:32 however we are still the 3rd in my timezone.

On 04/06/16 02:32, Chenxiao Zhao wrote:



On 6/3/2016 3:16 AM, Julien Grall wrote:

Hello,

On 03/06/16 18:05, Chenxiao Zhao wrote:

I finally found out that the problem is that the toolstack did not get
corret p2m_size while sending all pages on save(always be zero).
After I
fixed that, the guest could be restored but guest kernel caught
handle_mm_fault().

where do you think I'm going to investigate, guest kernel hibernation
restore or xen?


The hibernation support for ARM64 has only been merged recently in the
kernel. Which kernel are you using?


Hi Julien,

I'm using a linaro ported Linux kernel 4.1 for hikey from this link.

https://github.com/96boards/linux/tree/android-hikey-linaro-4.1

I also applied following patches to make the kernel support hibernation.


This looks the wrong way to do it as this series may requires some
patches which have been upstreamed before hand.

Linux upstream seems support to the hikey board [1]. Any reason to not
using it?


I tried a newer version of kernel 4.4, but got no luck to start dom0 with xen.
so I decide to stay in 4.1 for now.




[1] http://www.spinics.net/lists/arm-kernel/msg477769.html
[2] http://lists.xen.org/archives/html/xen-devel/2015-12/msg01068.html



Also, what are the modifications you have made to support Xen
suspend/resume for ARM64?


I believe I have posted my modifications on xen in the first mail of
this thread.


I mean in Linux. The patch from Ian Campbell does not have any kind of
support for ARM64.

For instance arch/arm/xen/suspend.c needs to be built for ARM64. So I am
wondering if your kernel has support of hibernation...


Oh, yes, I most forgot I added this file in arch/arm64/xen/Makefile to let it
build for arm64.




 From my understanding, a kernel hibernation will cause kernel to save
memories to disk(swap partition). But on guest save progress, the
hibernation for domU does not make the guest save memories to disk. it's
more like suspend all processes in guest, and memors actually depends on
xen toolstack to save the pages to file. Am I correct?


You are using an older tree with a patch series based on a newer tree.

So I would recommend you to move to a newer tree. If it is not possible,
please test that hibernation works on baremetal.


I think the suspend/resume in guest is working, cause I can use pause/unpause
command in toolstack to suspend/resume guest without problem. I can also see
the suspend/resume kernel messages from guest's console. The only problem is
it's can not resume from restore.


But can you still connect to the guest after resume, maybe over the network?
If you cannot, then something is likely wrong.


Hi Stefano,

I can connect to the guest after resume from xen console. It responds by 
'return' key, but I can not run any other commands, e.g. ls or ps. I 
think the guest is not 'fully' restored.






One thing that confused me is that the kernel's hibernation means the guest
kernel will save the memory state to disk and power off VM at last. The guest
will also take care of the memory restore itself. But I do not see the
save/restore on xen works that way. So my question is why it requires
hibernation (aka. suspend to disk) instead of the real suspend (aka. suspend
to RAM and standby)?


Xen suspend/resume has nothing to do with guest suspend to RAM or guest
hibernation.

Xen suspend/resume is a way for the hypervisor to save to file the
entire state of the VM, including RAM and the state of any devices.
Guest suspend to RAM and guest hibernation are two guest driven
technologies to save the state of the operating system to RAM or to
disk. The only link between Xen suspend and guest suspend is that when
Xen issues a domain suspend, it notifies the guest of it so that it can
ease the process.  The code in Linux to support Xen suspend/resume is:

drivers/xen/manage.c:do_suspend

and makes use of some of the Linux internal hooks provided for
hibernations (see CONFIG_HIBERNATE_CALLBACKS). But that's just for
better integration with the rest of Linux: hibernation is NOT what is
happening.

I hope that this clarifies things a bit, I realize that it is confusing.



Thanks for your explanation, It clear enough and just as my 
understanding from the code. I think the problem might caused by 
incompatible of arm p2m and xen save/restore mechanism. I'll try a 
core-dump and compare the memory after save and restore. I suppose this 
two dumps should be identical but there already pages are different. 
I'll let you know if I got some progress.


Regards.

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

[Xen-devel] [qemu-upstream-4.3-testing test] 95336: trouble: blocked/broken

[osstest service owner]

flight 95336 qemu-upstream-4.3-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/95336/

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64   3 host-install(3) broken REGR. vs. 80927
 build-i386-pvops  3 host-install(3) broken REGR. vs. 80927
 build-i3863 host-install(3) broken REGR. vs. 80927
 build-amd64-pvops 3 host-install(3) broken REGR. vs. 80927

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  1 build-check(1)blocked n/a
 build-amd64-libvirt   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-xl-qemuu-winxpsp3-vcpus1  1 build-check(1) blocked n/a
 test-amd64-i386-xl-raw1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-win7-amd64  1 build-check(1)  blocked n/a
 test-amd64-amd64-xl-credit2   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-i386-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-i386  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-amd64  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-win7-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-pygrub   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-winxpsp3  1 build-check(1)   blocked n/a
 test-amd64-amd64-xl-qcow2 1 build-check(1)   blocked  n/a
 test-amd64-amd64-amd64-pvgrub  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl   1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-debianhvm-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-qemuu-rhel6hvm-intel  1 build-check(1) blocked n/a
 test-amd64-i386-xl1 build-check(1)   blocked  n/a
 test-amd64-i386-qemuu-rhel6hvm-amd  1 build-check(1)   blocked n/a
 test-amd64-i386-pv1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-vhd  1 build-check(1)   blocked  n/a
 test-amd64-amd64-pair 1 build-check(1)   blocked  n/a
 test-amd64-amd64-pv   1 build-check(1)   blocked  n/a

version targeted for testing:
 qemuuc97c20f71240a538a19cb6b0e598bc1bbd5168f1
baseline version:
 qemuu10c1b763c26feb645627a1639e722515f3e1e876

Last test of basis80927  2016-02-06 13:30:02 Z  121 days
Testing same since93977  2016-05-10 11:09:16 Z   27 days  115 attempts


People who touched revisions under test:
  Gerd Hoffmann 
  Stefano Stabellini 

jobs:
 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  blocked 
 test-amd64-i386-xl   blocked 
 test-amd64-i386-qemuu-rhel6hvm-amd   blocked 
 test-amd64-amd64-xl-qemuu-debianhvm-amd64blocked 
 test-amd64-i386-xl-qemuu-debianhvm-amd64 blocked 
 test-amd64-i386-freebsd10-amd64  blocked 
 test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked 
 test-amd64-i386-xl-qemuu-ovmf-amd64  blocked 
 test-amd64-amd64-xl-qemuu-win7-amd64 blocked 
 test-amd64-i386-xl-qemuu-win7-amd64  blocked 
 test-amd64-amd64-xl-credit2  blocked 
 test-amd64-i386-freebsd10-i386   blocked 
 test-amd64-i386-qemuu-rhel6hvm-intel blocked 
 test-amd64-amd64-libvirt blocked 
 test-amd64-i386-libvirt  blocked 
 test-amd64-amd64-xl-multivcpublocked 
 

Re: [Xen-devel] [PATCH RESEND 00/14] Xen ARM DomU ACPI support

[Shannon Zhao]

On 2016/6/6 23:48, Julien Grall wrote:
> Hi Wei,
> 
> On 06/06/16 13:26, Wei Liu wrote:
>> On Tue, May 31, 2016 at 01:02:52PM +0800, Shannon Zhao wrote:
>>> From: Shannon Zhao 
>>>
>>> The design of this feature is described as below.
>>> Firstly, the toolstack (libxl) generates the ACPI tables according the
>>> number of vcpus and gic controller.
>>>
>>> Then, it copies these ACPI tables to DomU memory space and passes
>>> them to UEFI firmware through the "ARM multiboot" protocol.
>>>
>>> At last, UEFI gets the ACPI tables through the "ARM multiboot" protocol
>>> and installs these tables like the usual way and passes both ACPI and DT
>>> information to the Xen DomU.
>>>
>>> Currently libxl only generates RSDP, XSDT, GTDT, MADT, FADT, DSDT tables
>>> since it's enough now.
>>>
>>> This has been tested using guest kernel with the Dom0 ACPI support
>>> patches which could be fetched from:
>>> https://git.kernel.org/cgit/linux/kernel/git/mfleming/efi.git/log/?h=efi/arm-xen
>>>
>>>
>>> Shannon Zhao (14):
>>>libxl/arm: Fix the function name in error log
>>>libxl/arm: Factor out codes for generating DTB
>>>libxc: Add placeholders for ACPI tables blob and size
>>>tools: add ACPI tables relevant definitions
>>>libxl/arm: Construct ACPI GTDT table
>>>libxl/arm: Construct ACPI FADT table
>>>libxl/arm: Construct ACPI DSDT table
>>>libxl/arm: Construct ACPI MADT table
>>>libxl/arm: Construct ACPI XSDT table
>>>libxl/arm: Construct ACPI RSDP table
>>>libxl/arm: Initialize domain param HVM_PARAM_CALLBACK_IRQ
>>>libxl/arm: Add ACPI module
>>>libxl/arm: initialize memory information of ACPI blob
>>>libxc/xc_dom_core: Copy ACPI tables to guest memory space
>>>
>>
>> After going through this series, I think the code mostly looks good.
>>
>> There is one higher level question: you seem to have put a lot of the
>> table construction code in libxl, while I failed to see why specific
>> libxl information is needed. Have you considered moving the code to
>> libxc?
> 
> I would rather avoid to have the device tree built by libxl and ACPI
> tables built by libxc.
> 
> I don't remember why we have decided to implement DT building in libxl,
> I guess it is because we need to know which GIC version is used (GICv2
> vs GICv3).
> 
> In the long run, we might also need to have more part of the firmware
> configurable (performance monitor support, memory layout, UART...).
> 
> Most of those information are available easily in libxl, we would to
> export them of libxc.
Yes, and one more reason is that to support ACPI, it also needs to add
the ACPI multiboot module in DT.

Thanks,
-- 
Shannon


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

Re: [Xen-devel] [PATCH] Revert "x86/hvm: add support for pcommit instruction"

[Tian, Kevin]

> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: Monday, June 06, 2016 6:38 PM
> 
> >>> On 06.06.16 at 12:31,  wrote:
> > On Mon, Jun 06, 2016 at 02:54:47AM -0600, Jan Beulich wrote:
> >> >>> On 06.06.16 at 07:56,  wrote:
> >> > This reverts commit cfacce340608be5f94ce0c8f424487b63c3d5399.
> >> >
> >> > Platforms supporting Intel NVDIMM are now required to provide
> >> > persistency once pmem stores are accepted by the memory subsystem.
> >> > This is usually achieved by a platform-level feature known as ADR
> >> > (Asynchronous DRAM Refresh) that flushes any memory subsystem write
> >> > pending queues on power loss/shutdown. Therefore, the pcommit
> >> > instruction, which has not yet shipped on any product (and will not),
> >> > is no longer needed and is deprecated.
> >> >
> >> > Signed-off-by: Haozhong Zhang 
> >>
> >> Acked-by: Jan Beulich 
> >>
> >> Wei - certainly something to consider for 4.7.
> >>
> >
> > Sure. No point in shipping feature that won't show up in hardware.
> >
> > Release-acked-by: Wei Liu 
> >
> > But please wait a bit before committing while we try to sort out RC5 and
> > branching etc.
> 
> Sure. Got to wait for a VMX maintainer ack anyway.
> 

Here it is:

Acked-by: Kevin Tian 

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

Re: [Xen-devel] Basic bare metal ARM domain interface

[Ivan Pavic]

Hello Julien,

On Fri, Jun 03, 2016 at 11:23:33AM +0100, Julien Grall wrote:
> 
> 
> On 02/06/16 20:09, Ivan Pavic wrote:
> >Hello Julien,
> 
> Hello Ivan,
> 
> >On Thu, Jun 02, 2016 at 12:41:02PM +0100, Julien Grall wrote:
> >>
> >>Which compiler led to use a data abort? And where was the data
> >>abort? In Xen or the guest?
> >>
> >
> >When I compile app directly on ODROID I used:
> >arm-linux-gnueabihf
> >gcc version 4.8.4 (Ubuntu/Linaro 4.8.4-2ubuntu1~14.04.1)
> >
> >HVC instruction works form assembler, but when I call it from main, for
> >example:
> >HYPERVISOR_console_io (HYPERCALL_WRITE, SYSTEM_UP_LEN, SYSTEM_UP_MSG);
> >then I get this in xl dmesg:
> >traps.c:2450:d2v0 HSR=0x9046 pc=0x40008250 gva=0 gpa=
> 
> To be honest this looks like a latent issue in your app rather than
> a bug in the compiler.
> 
> I would try to find which instruction is causing the data abort.
> Also is your app running with page table and cache enabled?
> 

Page table and cache are related to MMU, right? I didn't initialize MMU 
so far.

Regards,

Ivan Pavic



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

[Xen-devel] [xen-4.6-testing test] 95328: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-xl-credit2  15 guest-start/debian.repeat fail REGR. vs. 95235

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-rumpuserxen-amd64 15 
rumpuserxen-demo-xenstorels/xenstorels.repeat fail REGR. vs. 95235
 test-armhf-armhf-xl-rtds15 guest-start/debian.repeat fail blocked in 95235
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 95235
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail like 95235
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 95235
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 95235

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass
 test-armhf-armhf-xl-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  2805844bf20e1cae824d0ccc864e46dfa8f134da
baseline version:
 xen  f354fb4670132c9811b433ba34fb8edd46f7

Last test of basis95235  2016-06-03 10:55:03 Z3 days
Testing same since95328  2016-06-06 13:42:21 Z0 days1 attempts


People who touched revisions under test:
  Ian Jackson 

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

[Xen-devel] [PATCH V6 3/3] arm/acpi: Add Server Base System Architecture UART support

[Shanker Donthineni]

The ARM Server Base System Architecture describes a generic UART
interface. It doesn't support clock control registers, modem
control, DMA and hardware flow control features. So, extend the
driver probe() to handle SBSA interface and skip the accessing
PL011 registers that are not described in SBSA document
(ARM-DEN-0029 Version 3.0, 6 APPENDIX B: GENERIC UART).

Signed-off-by: Shanker Donthineni 
---
Changes since v3:
  Moved non-SBSA related changes to patches 1/3 and 2/3.

changes since v2:
  Edited commit text to include SBSA document version.
  Remove setting baudrate code completely as per Julien's suggestion.
  Support both the SBSA interface types ACPI_DBG2_SBSA & ACPI_DBG2_SBSA_32.
  Replace MIS references with combination of RIS & IMSC. 

Changes since v1:
  Don't access UART registers that are not part of SBSA document.
  Move setting baudrate function to a separate function.

 xen/drivers/char/pl011.c | 54 ++--
 1 file changed, 39 insertions(+), 15 deletions(-)

diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
index 755a965..b8ba30e 100644
--- a/xen/drivers/char/pl011.c
+++ b/xen/drivers/char/pl011.c
@@ -41,6 +41,7 @@ static struct pl011 {
 /* struct timer timer; */
 /* unsigned int timeout_ms; */
 /* bool_t probing, intr_works; */
+bool sbsa;  /* ARM SBSA generic interface */
 } pl011_com = {0};
 
 /* These parity settings can be ORed directly into the LCR. */
@@ -92,14 +93,17 @@ static void __init pl011_init_preirq(struct serial_port 
*port)
 /* No interrupts, please. */
 pl011_write(uart, IMSC, 0);
 
-/* Definitely no DMA */
-pl011_write(uart, DMACR, 0x0);
-
-/* This write must follow FBRD and IBRD writes. */
-pl011_write(uart, LCR_H, (uart->data_bits - 5) << 5
-| FEN
-| ((uart->stop_bits - 1) << 3)
-| uart->parity);
+if ( !uart->sbsa )
+{
+/* Definitely no DMA */
+pl011_write(uart, DMACR, 0x0);
+
+/* This write must follow FBRD and IBRD writes. */
+pl011_write(uart, LCR_H, (uart->data_bits - 5) << 5
+| FEN
+| ((uart->stop_bits - 1) << 3)
+| uart->parity);
+}
 /* Clear errors */
 pl011_write(uart, RSR, 0);
 
@@ -107,10 +111,13 @@ static void __init pl011_init_preirq(struct serial_port 
*port)
 pl011_write(uart, IMSC, 0);
 pl011_write(uart, ICR, ALLI);
 
-/* Enable the UART for RX and TX; keep RTS and DTR */
-cr = pl011_read(uart, CR);
-cr &= RTS | DTR;
-pl011_write(uart, CR, cr | RXE | TXE | UARTEN);
+if ( !uart->sbsa )
+{
+/* Enable the UART for RX and TX; keep RTS and DTR */
+cr = pl011_read(uart, CR);
+cr &= RTS | DTR;
+pl011_write(uart, CR, cr | RXE | TXE | UARTEN);
+}
 }
 
 static void __init pl011_init_postirq(struct serial_port *port)
@@ -212,7 +219,7 @@ static struct uart_driver __read_mostly pl011_driver = {
 .vuart_info   = pl011_vuart,
 };
 
-static int __init pl011_uart_init(int irq, u64 addr, u64 size)
+static int __init pl011_uart_init(int irq, u64 addr, u64 size, bool sbsa)
 {
 struct pl011 *uart;
 
@@ -221,6 +228,7 @@ static int __init pl011_uart_init(int irq, u64 addr, u64 
size)
 uart->data_bits = 8;
 uart->parity= PARITY_NONE;
 uart->stop_bits = 1;
+uart->sbsa  = sbsa;
 
 uart->regs = ioremap_nocache(addr, size);
 if ( !uart->regs )
@@ -269,7 +277,7 @@ static int __init pl011_dt_uart_init(struct dt_device_node 
*dev,
 return -EINVAL;
 }
 
-res = pl011_uart_init(res, addr, size);
+res = pl011_uart_init(res, addr, size, false);
 if ( res < 0 )
 {
 printk("pl011: Unable to initialize\n");
@@ -300,6 +308,7 @@ static int __init pl011_acpi_uart_init(const void *data)
 acpi_status status;
 struct acpi_table_spcr *spcr = NULL;
 int res;
+bool sbsa = false;
 
 status = acpi_get_table(ACPI_SIG_SPCR, 0,
 (struct acpi_table_header **));
@@ -310,11 +319,15 @@ static int __init pl011_acpi_uart_init(const void *data)
 return -EINVAL;
 }
 
+if ( (spcr->interface_type == ACPI_DBG2_SBSA) ||
+ (spcr->interface_type == ACPI_DBG2_SBSA_32) )
+sbsa = true;
+
 /* trigger/polarity information is not available in spcr */
 irq_set_type(spcr->interrupt, IRQ_TYPE_LEVEL_HIGH);
 
 res = pl011_uart_init(spcr->interrupt, spcr->serial_port.address,
-  PAGE_SIZE);
+  PAGE_SIZE, sbsa);
 if ( res < 0 )
 {
 printk("pl011: Unable to initialize\n");
@@ -328,6 +341,17 @@ ACPI_DEVICE_START(apl011, "PL011 UART", DEVICE_SERIAL)
 .class_type = ACPI_DBG2_PL011,
 .init = pl011_acpi_uart_init,
 ACPI_DEVICE_END
+
+ACPI_DEVICE_START(asbsa_uart, "SBSA UART", 

[Xen-devel] [PATCH V6 1/3] drivers/pl011: Don't configure baudrate

[Shanker Donthineni]

The default baud and clock_hz configuration parameters are hardcoded
(commit 60ff980995008caf) for Versatile Express. Other platforms,
these default values may not be valid and might cause problems by
programming registers IBRD and FBRD incorrectly.

So, removing driver logic that sets the baudrate to fix the problem.
The boot firmware is responsible for setting the correct baudrate.

Signed-off-by: Shanker Donthineni 
---
 xen/drivers/char/pl011.c | 21 +
 1 file changed, 1 insertion(+), 20 deletions(-)

diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
index 1212d5c..6a3c21b 100644
--- a/xen/drivers/char/pl011.c
+++ b/xen/drivers/char/pl011.c
@@ -31,7 +31,7 @@
 #include 
 
 static struct pl011 {
-unsigned int baud, clock_hz, data_bits, parity, stop_bits;
+unsigned int data_bits, parity, stop_bits;
 unsigned int irq;
 void __iomem *regs;
 /* UART with IRQ line: interrupt-driven I/O. */
@@ -84,7 +84,6 @@ static void pl011_interrupt(int irq, void *data, struct 
cpu_user_regs *regs)
 static void __init pl011_init_preirq(struct serial_port *port)
 {
 struct pl011 *uart = port->uart;
-unsigned int divisor;
 unsigned int cr;
 
 /* No interrupts, please. */
@@ -93,22 +92,6 @@ static void __init pl011_init_preirq(struct serial_port 
*port)
 /* Definitely no DMA */
 pl011_write(uart, DMACR, 0x0);
 
-/* Line control and baud-rate generator. */
-if ( uart->baud != BAUD_AUTO )
-{
-/* Baud rate specified: program it into the divisor latch. */
-divisor = (uart->clock_hz << 2) / uart->baud; /* clk << 6 / bd << 4 */
-pl011_write(uart, FBRD, divisor & 0x3f);
-pl011_write(uart, IBRD, divisor >> 6);
-}
-else
-{
-/* Baud rate already set: read it out from the divisor latch. */
-divisor = (pl011_read(uart, IBRD) << 6) | (pl011_read(uart, FBRD));
-if (!divisor)
-panic("pl011: No Baud rate configured\n");
-uart->baud = (uart->clock_hz << 2) / divisor;
-}
 /* This write must follow FBRD and IBRD writes. */
 pl011_write(uart, LCR_H, (uart->data_bits - 5) << 5
 | FEN
@@ -232,8 +215,6 @@ static int __init pl011_uart_init(int irq, u64 addr, u64 
size)
 
 uart = _com;
 uart->irq   = irq;
-uart->clock_hz  = 0x16e3600;
-uart->baud  = BAUD_AUTO;
 uart->data_bits = 8;
 uart->parity= PARITY_NONE;
 uart->stop_bits = 1;
-- 
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project


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

[Xen-devel] [PATCH V6 2/3] drivers/pl011: Use combination of UARTRIS and UARTMSC instead of UARTMIS

[Shanker Donthineni]

The Masked interrupt status register (UARTMIS) is not described in ARM
SBSA 2.x document. Anding of two registers UARTMSC and UARTRIS values
gives the same information as register UARTMIS.

UARTRIS, UARTMSC and UARTMIS definitions are found in PrimeCell UART
PL011 (Revision: r1p4).
 - 3.3.10 Interrupt mask set/clear register, UARTIMSC
 - 3.3.11 Raw interrupt status register, UARTRIS
 - 3.3.12 Masked interrupt status register, UARTMIS

This change is necessary for driver to be SBSA compliant v2.x without
affecting the current driver functionality.

Signed-off-by: Shanker Donthineni 
---
  Fixed typo in commit text.

 xen/drivers/char/pl011.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
index 6a3c21b..755a965 100644
--- a/xen/drivers/char/pl011.c
+++ b/xen/drivers/char/pl011.c
@@ -57,7 +57,10 @@ static void pl011_interrupt(int irq, void *data, struct 
cpu_user_regs *regs)
 {
 struct serial_port *port = data;
 struct pl011 *uart = port->uart;
-unsigned int status = pl011_read(uart, MIS);
+unsigned int status;
+
+/* UARTMIS is not documented in SBSA v2.x, so using UARTRIS/UARTIMSC */
+status = pl011_read(uart, RIS) & pl011_read(uart, IMSC);
 
 if ( status )
 {
@@ -76,7 +79,7 @@ static void pl011_interrupt(int irq, void *data, struct 
cpu_user_regs *regs)
 if ( status & (TXI) )
 serial_tx_interrupt(port, regs);
 
-status = pl011_read(uart, MIS);
+status = pl011_read(uart, RIS) & pl011_read(uart, IMSC);
 } while (status != 0);
 }
 }
-- 
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project


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

[Xen-devel] [PATCH V5 3/3] arm/acpi: Add Server Base System Architecture UART support

[Shanker Donthineni]

The ARM Server Base System Architecture describes a generic UART
interface. It doesn't support clock control registers, modem
control, DMA and hardware flow control features. So, extend the
driver probe() to handle SBSA interface and skip the accessing
PL011 registers that are not described in SBSA document
(ARM-DEN-0029 Version 3.0, 6 APPENDIX B: GENERIC UART).

Signed-off-by: Shanker Donthineni 
---
Changes since v3:
  Moved non-SBSA related changes to patches 1/3 and 2/3.

changes since v2:
  Edited commit text to include SBSA document version.
  Remove setting baudrate code completely as per Julien's suggestion.
  Support both the SBSA interface types ACPI_DBG2_SBSA & ACPI_DBG2_SBSA_32.
  Replace MIS references with combination of RIS & IMSC. 

Changes since v1:
  Don't access UART registers that are not part of SBSA document.
  Move setting baudrate function to a separate function.

 xen/drivers/char/pl011.c | 54 ++--
 1 file changed, 39 insertions(+), 15 deletions(-)

diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
index 755a965..b8ba30e 100644
--- a/xen/drivers/char/pl011.c
+++ b/xen/drivers/char/pl011.c
@@ -41,6 +41,7 @@ static struct pl011 {
 /* struct timer timer; */
 /* unsigned int timeout_ms; */
 /* bool_t probing, intr_works; */
+bool sbsa;  /* ARM SBSA generic interface */
 } pl011_com = {0};
 
 /* These parity settings can be ORed directly into the LCR. */
@@ -92,14 +93,17 @@ static void __init pl011_init_preirq(struct serial_port 
*port)
 /* No interrupts, please. */
 pl011_write(uart, IMSC, 0);
 
-/* Definitely no DMA */
-pl011_write(uart, DMACR, 0x0);
-
-/* This write must follow FBRD and IBRD writes. */
-pl011_write(uart, LCR_H, (uart->data_bits - 5) << 5
-| FEN
-| ((uart->stop_bits - 1) << 3)
-| uart->parity);
+if ( !uart->sbsa )
+{
+/* Definitely no DMA */
+pl011_write(uart, DMACR, 0x0);
+
+/* This write must follow FBRD and IBRD writes. */
+pl011_write(uart, LCR_H, (uart->data_bits - 5) << 5
+| FEN
+| ((uart->stop_bits - 1) << 3)
+| uart->parity);
+}
 /* Clear errors */
 pl011_write(uart, RSR, 0);
 
@@ -107,10 +111,13 @@ static void __init pl011_init_preirq(struct serial_port 
*port)
 pl011_write(uart, IMSC, 0);
 pl011_write(uart, ICR, ALLI);
 
-/* Enable the UART for RX and TX; keep RTS and DTR */
-cr = pl011_read(uart, CR);
-cr &= RTS | DTR;
-pl011_write(uart, CR, cr | RXE | TXE | UARTEN);
+if ( !uart->sbsa )
+{
+/* Enable the UART for RX and TX; keep RTS and DTR */
+cr = pl011_read(uart, CR);
+cr &= RTS | DTR;
+pl011_write(uart, CR, cr | RXE | TXE | UARTEN);
+}
 }
 
 static void __init pl011_init_postirq(struct serial_port *port)
@@ -212,7 +219,7 @@ static struct uart_driver __read_mostly pl011_driver = {
 .vuart_info   = pl011_vuart,
 };
 
-static int __init pl011_uart_init(int irq, u64 addr, u64 size)
+static int __init pl011_uart_init(int irq, u64 addr, u64 size, bool sbsa)
 {
 struct pl011 *uart;
 
@@ -221,6 +228,7 @@ static int __init pl011_uart_init(int irq, u64 addr, u64 
size)
 uart->data_bits = 8;
 uart->parity= PARITY_NONE;
 uart->stop_bits = 1;
+uart->sbsa  = sbsa;
 
 uart->regs = ioremap_nocache(addr, size);
 if ( !uart->regs )
@@ -269,7 +277,7 @@ static int __init pl011_dt_uart_init(struct dt_device_node 
*dev,
 return -EINVAL;
 }
 
-res = pl011_uart_init(res, addr, size);
+res = pl011_uart_init(res, addr, size, false);
 if ( res < 0 )
 {
 printk("pl011: Unable to initialize\n");
@@ -300,6 +308,7 @@ static int __init pl011_acpi_uart_init(const void *data)
 acpi_status status;
 struct acpi_table_spcr *spcr = NULL;
 int res;
+bool sbsa = false;
 
 status = acpi_get_table(ACPI_SIG_SPCR, 0,
 (struct acpi_table_header **));
@@ -310,11 +319,15 @@ static int __init pl011_acpi_uart_init(const void *data)
 return -EINVAL;
 }
 
+if ( (spcr->interface_type == ACPI_DBG2_SBSA) ||
+ (spcr->interface_type == ACPI_DBG2_SBSA_32) )
+sbsa = true;
+
 /* trigger/polarity information is not available in spcr */
 irq_set_type(spcr->interrupt, IRQ_TYPE_LEVEL_HIGH);
 
 res = pl011_uart_init(spcr->interrupt, spcr->serial_port.address,
-  PAGE_SIZE);
+  PAGE_SIZE, sbsa);
 if ( res < 0 )
 {
 printk("pl011: Unable to initialize\n");
@@ -328,6 +341,17 @@ ACPI_DEVICE_START(apl011, "PL011 UART", DEVICE_SERIAL)
 .class_type = ACPI_DBG2_PL011,
 .init = pl011_acpi_uart_init,
 ACPI_DEVICE_END
+
+ACPI_DEVICE_START(asbsa_uart, "SBSA UART", 

[Xen-devel] [PATCH V5 1/3] drivers/pl011: Don't configure baudrate

[Shanker Donthineni]

The default baud and clock_hz configuration parameters are hardcoded
(commit 60ff980995008caf) for Versatile Express. Other platforms,
these default values may not be valid and might cause problems by
programming registers IBRD and FBRD incorrectly.

So, removing driver logic that sets the baudrate to fix the problem.
The boot firmware is responsible for setting the correct baudrate.

Signed-off-by: Shanker Donthineni 
---
 xen/drivers/char/pl011.c | 21 +
 1 file changed, 1 insertion(+), 20 deletions(-)

diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
index 1212d5c..6a3c21b 100644
--- a/xen/drivers/char/pl011.c
+++ b/xen/drivers/char/pl011.c
@@ -31,7 +31,7 @@
 #include 
 
 static struct pl011 {
-unsigned int baud, clock_hz, data_bits, parity, stop_bits;
+unsigned int data_bits, parity, stop_bits;
 unsigned int irq;
 void __iomem *regs;
 /* UART with IRQ line: interrupt-driven I/O. */
@@ -84,7 +84,6 @@ static void pl011_interrupt(int irq, void *data, struct 
cpu_user_regs *regs)
 static void __init pl011_init_preirq(struct serial_port *port)
 {
 struct pl011 *uart = port->uart;
-unsigned int divisor;
 unsigned int cr;
 
 /* No interrupts, please. */
@@ -93,22 +92,6 @@ static void __init pl011_init_preirq(struct serial_port 
*port)
 /* Definitely no DMA */
 pl011_write(uart, DMACR, 0x0);
 
-/* Line control and baud-rate generator. */
-if ( uart->baud != BAUD_AUTO )
-{
-/* Baud rate specified: program it into the divisor latch. */
-divisor = (uart->clock_hz << 2) / uart->baud; /* clk << 6 / bd << 4 */
-pl011_write(uart, FBRD, divisor & 0x3f);
-pl011_write(uart, IBRD, divisor >> 6);
-}
-else
-{
-/* Baud rate already set: read it out from the divisor latch. */
-divisor = (pl011_read(uart, IBRD) << 6) | (pl011_read(uart, FBRD));
-if (!divisor)
-panic("pl011: No Baud rate configured\n");
-uart->baud = (uart->clock_hz << 2) / divisor;
-}
 /* This write must follow FBRD and IBRD writes. */
 pl011_write(uart, LCR_H, (uart->data_bits - 5) << 5
 | FEN
@@ -232,8 +215,6 @@ static int __init pl011_uart_init(int irq, u64 addr, u64 
size)
 
 uart = _com;
 uart->irq   = irq;
-uart->clock_hz  = 0x16e3600;
-uart->baud  = BAUD_AUTO;
 uart->data_bits = 8;
 uart->parity= PARITY_NONE;
 uart->stop_bits = 1;
-- 
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project


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

[Xen-devel] [PATCH V5 2/3] drivers/pl011: Use combination of UARTRIS and UARTMSC instead of UARTMIS

[Shanker Donthineni]

The Masked interrupt status register (UARTMIS) is not described in ARM
SBSA 2.x document. Anding of two registers UARTMSC and UARTRIS values
gives the same information as register UARTMIS.

UARTRIS, UARTMSC and UARTMIS definitions are found in PrimeCell UART
PL011 (Revision: r1p4).
 - 3.3.10 Interrupt mask set/clear register, UARTIMSC
 - 3.3.11 Raw interrupt status register, UARTRIS
 - 3.3.12 Masked interrupt status register, UARTMIS

This change is necessary for driver to be SBSA compliant v2.x without
afftecting the current driver functionality.

Signed-off-by: Shanker Donthineni 
---
  Fixed typo in commit text.

 xen/drivers/char/pl011.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
index 6a3c21b..755a965 100644
--- a/xen/drivers/char/pl011.c
+++ b/xen/drivers/char/pl011.c
@@ -57,7 +57,10 @@ static void pl011_interrupt(int irq, void *data, struct 
cpu_user_regs *regs)
 {
 struct serial_port *port = data;
 struct pl011 *uart = port->uart;
-unsigned int status = pl011_read(uart, MIS);
+unsigned int status;
+
+/* UARTMIS is not documented in SBSA v2.x, so using UARTRIS/UARTIMSC */
+status = pl011_read(uart, RIS) & pl011_read(uart, IMSC);
 
 if ( status )
 {
@@ -76,7 +79,7 @@ static void pl011_interrupt(int irq, void *data, struct 
cpu_user_regs *regs)
 if ( status & (TXI) )
 serial_tx_interrupt(port, regs);
 
-status = pl011_read(uart, MIS);
+status = pl011_read(uart, RIS) & pl011_read(uart, IMSC);
 } while (status != 0);
 }
 }
-- 
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project


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

[Xen-devel] [PATCH V4 3/3] arm/acpi: Add Server Base System Architecture UART support

[Shanker Donthineni]

The ARM Server Base System Architecture describes a generic UART
interface. It doesn't support clock control registers, modem
control, DMA and hardware flow control features. So, extend the
driver probe() to handle SBSA interface and skip the accessing
PL011 registers that are not described in SBSA document
(ARM-DEN-0029 Version 3.0, 6 APPENDIX B: GENERIC UART).

Signed-off-by: Shanker Donthineni 
---
Changes since v3:
  Moved non-SBSA related changes to patches 1/3 and 2/3.

changes since v2:
  Edited commit text to include SBSA document version.
  Remove setting baudrate code completely as per Julien's suggestion.
  Support both the SBSA interface types ACPI_DBG2_SBSA & ACPI_DBG2_SBSA_32.
  Replace MIS references with combination of RIS & IMSC. 

Changes since v1:
  Don't access UART registers that are not part of SBSA document.
  Move setting baudrate function to a separate function.

 xen/drivers/char/pl011.c | 54 ++--
 1 file changed, 39 insertions(+), 15 deletions(-)

diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
index 755a965..b8ba30e 100644
--- a/xen/drivers/char/pl011.c
+++ b/xen/drivers/char/pl011.c
@@ -41,6 +41,7 @@ static struct pl011 {
 /* struct timer timer; */
 /* unsigned int timeout_ms; */
 /* bool_t probing, intr_works; */
+bool sbsa;  /* ARM SBSA generic interface */
 } pl011_com = {0};
 
 /* These parity settings can be ORed directly into the LCR. */
@@ -92,14 +93,17 @@ static void __init pl011_init_preirq(struct serial_port 
*port)
 /* No interrupts, please. */
 pl011_write(uart, IMSC, 0);
 
-/* Definitely no DMA */
-pl011_write(uart, DMACR, 0x0);
-
-/* This write must follow FBRD and IBRD writes. */
-pl011_write(uart, LCR_H, (uart->data_bits - 5) << 5
-| FEN
-| ((uart->stop_bits - 1) << 3)
-| uart->parity);
+if ( !uart->sbsa )
+{
+/* Definitely no DMA */
+pl011_write(uart, DMACR, 0x0);
+
+/* This write must follow FBRD and IBRD writes. */
+pl011_write(uart, LCR_H, (uart->data_bits - 5) << 5
+| FEN
+| ((uart->stop_bits - 1) << 3)
+| uart->parity);
+}
 /* Clear errors */
 pl011_write(uart, RSR, 0);
 
@@ -107,10 +111,13 @@ static void __init pl011_init_preirq(struct serial_port 
*port)
 pl011_write(uart, IMSC, 0);
 pl011_write(uart, ICR, ALLI);
 
-/* Enable the UART for RX and TX; keep RTS and DTR */
-cr = pl011_read(uart, CR);
-cr &= RTS | DTR;
-pl011_write(uart, CR, cr | RXE | TXE | UARTEN);
+if ( !uart->sbsa )
+{
+/* Enable the UART for RX and TX; keep RTS and DTR */
+cr = pl011_read(uart, CR);
+cr &= RTS | DTR;
+pl011_write(uart, CR, cr | RXE | TXE | UARTEN);
+}
 }
 
 static void __init pl011_init_postirq(struct serial_port *port)
@@ -212,7 +219,7 @@ static struct uart_driver __read_mostly pl011_driver = {
 .vuart_info   = pl011_vuart,
 };
 
-static int __init pl011_uart_init(int irq, u64 addr, u64 size)
+static int __init pl011_uart_init(int irq, u64 addr, u64 size, bool sbsa)
 {
 struct pl011 *uart;
 
@@ -221,6 +228,7 @@ static int __init pl011_uart_init(int irq, u64 addr, u64 
size)
 uart->data_bits = 8;
 uart->parity= PARITY_NONE;
 uart->stop_bits = 1;
+uart->sbsa  = sbsa;
 
 uart->regs = ioremap_nocache(addr, size);
 if ( !uart->regs )
@@ -269,7 +277,7 @@ static int __init pl011_dt_uart_init(struct dt_device_node 
*dev,
 return -EINVAL;
 }
 
-res = pl011_uart_init(res, addr, size);
+res = pl011_uart_init(res, addr, size, false);
 if ( res < 0 )
 {
 printk("pl011: Unable to initialize\n");
@@ -300,6 +308,7 @@ static int __init pl011_acpi_uart_init(const void *data)
 acpi_status status;
 struct acpi_table_spcr *spcr = NULL;
 int res;
+bool sbsa = false;
 
 status = acpi_get_table(ACPI_SIG_SPCR, 0,
 (struct acpi_table_header **));
@@ -310,11 +319,15 @@ static int __init pl011_acpi_uart_init(const void *data)
 return -EINVAL;
 }
 
+if ( (spcr->interface_type == ACPI_DBG2_SBSA) ||
+ (spcr->interface_type == ACPI_DBG2_SBSA_32) )
+sbsa = true;
+
 /* trigger/polarity information is not available in spcr */
 irq_set_type(spcr->interrupt, IRQ_TYPE_LEVEL_HIGH);
 
 res = pl011_uart_init(spcr->interrupt, spcr->serial_port.address,
-  PAGE_SIZE);
+  PAGE_SIZE, sbsa);
 if ( res < 0 )
 {
 printk("pl011: Unable to initialize\n");
@@ -328,6 +341,17 @@ ACPI_DEVICE_START(apl011, "PL011 UART", DEVICE_SERIAL)
 .class_type = ACPI_DBG2_PL011,
 .init = pl011_acpi_uart_init,
 ACPI_DEVICE_END
+
+ACPI_DEVICE_START(asbsa_uart, "SBSA UART", 

[Xen-devel] [PATCH V4 1/3] drivers/pl011: Don't configure baudrate

[Shanker Donthineni]

The default baud and clock_hz configuration parameters are hardcoded
(commit 60ff980995008caf) for Versatile Express. Other platforms,
these default values may not be valid and might cause problems by
programming registers IBRD and FBRD incorrectly.

So, removing driver logic that sets the baudrate to fix the problem.
The boot firmware is responsible for setting the correct baudrate.

Signed-off-by: Shanker Donthineni 
---
 xen/drivers/char/pl011.c | 21 +
 1 file changed, 1 insertion(+), 20 deletions(-)

diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
index 1212d5c..6a3c21b 100644
--- a/xen/drivers/char/pl011.c
+++ b/xen/drivers/char/pl011.c
@@ -31,7 +31,7 @@
 #include 
 
 static struct pl011 {
-unsigned int baud, clock_hz, data_bits, parity, stop_bits;
+unsigned int data_bits, parity, stop_bits;
 unsigned int irq;
 void __iomem *regs;
 /* UART with IRQ line: interrupt-driven I/O. */
@@ -84,7 +84,6 @@ static void pl011_interrupt(int irq, void *data, struct 
cpu_user_regs *regs)
 static void __init pl011_init_preirq(struct serial_port *port)
 {
 struct pl011 *uart = port->uart;
-unsigned int divisor;
 unsigned int cr;
 
 /* No interrupts, please. */
@@ -93,22 +92,6 @@ static void __init pl011_init_preirq(struct serial_port 
*port)
 /* Definitely no DMA */
 pl011_write(uart, DMACR, 0x0);
 
-/* Line control and baud-rate generator. */
-if ( uart->baud != BAUD_AUTO )
-{
-/* Baud rate specified: program it into the divisor latch. */
-divisor = (uart->clock_hz << 2) / uart->baud; /* clk << 6 / bd << 4 */
-pl011_write(uart, FBRD, divisor & 0x3f);
-pl011_write(uart, IBRD, divisor >> 6);
-}
-else
-{
-/* Baud rate already set: read it out from the divisor latch. */
-divisor = (pl011_read(uart, IBRD) << 6) | (pl011_read(uart, FBRD));
-if (!divisor)
-panic("pl011: No Baud rate configured\n");
-uart->baud = (uart->clock_hz << 2) / divisor;
-}
 /* This write must follow FBRD and IBRD writes. */
 pl011_write(uart, LCR_H, (uart->data_bits - 5) << 5
 | FEN
@@ -232,8 +215,6 @@ static int __init pl011_uart_init(int irq, u64 addr, u64 
size)
 
 uart = _com;
 uart->irq   = irq;
-uart->clock_hz  = 0x16e3600;
-uart->baud  = BAUD_AUTO;
 uart->data_bits = 8;
 uart->parity= PARITY_NONE;
 uart->stop_bits = 1;
-- 
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project


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

[Xen-devel] [PATCH V4 2/3] drivers/pl011: Use combination of UARTRIS and UARTMSC instead of UARTMIS

[Shanker Donthineni]

The Masked interrupt status register (UARTMIS) is not described in ARM
SBSA 2.x document. Anding of two registers UARTMSC and UARTRIS values
gives the same information as register UARTMIS.

UARTRIS, UARTMSC and UARTMIS definitions are found in PrimeCell UART
PL011 (Revision: r1p4).
 - 3.3.10 Interrupt mask set/clear register, UARTIMSC
 - 3.3.11 Raw interrupt status register, UARTRIS
 - 3.3.12 Masked interrupt status register, UARTMIS

This this necessary for driver to be SBSA compliant v2.x without
afftecting the current driver functionality.

Signed-off-by: Shanker Donthineni 
---
 xen/drivers/char/pl011.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
index 6a3c21b..755a965 100644
--- a/xen/drivers/char/pl011.c
+++ b/xen/drivers/char/pl011.c
@@ -57,7 +57,10 @@ static void pl011_interrupt(int irq, void *data, struct 
cpu_user_regs *regs)
 {
 struct serial_port *port = data;
 struct pl011 *uart = port->uart;
-unsigned int status = pl011_read(uart, MIS);
+unsigned int status;
+
+/* UARTMIS is not documented in SBSA v2.x, so using UARTRIS/UARTIMSC */
+status = pl011_read(uart, RIS) & pl011_read(uart, IMSC);
 
 if ( status )
 {
@@ -76,7 +79,7 @@ static void pl011_interrupt(int irq, void *data, struct 
cpu_user_regs *regs)
 if ( status & (TXI) )
 serial_tx_interrupt(port, regs);
 
-status = pl011_read(uart, MIS);
+status = pl011_read(uart, RIS) & pl011_read(uart, IMSC);
 } while (status != 0);
 }
 }
-- 
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project


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

[Xen-devel] libxenvchan license (lGPLv2.1) and xen/include/public/COPYING license?

[Konrad Rzeszutek Wilk]

Hey!

I picked the list of people on this email from the git log on 
tools/misc/libvchan/
(albeit this is about xen/include/public/io/libxenvchan.h).

The problem is that the 'COPYING' file in the root directory of Xen source 
says:

"   
   
Licensing Exceptions (the relaxed BSD-style license)
   

   

   
For the convenience of users and those who are porting OSes to run as   
   
Xen guests, certain files in this repository are not subject to the 
   
GPL when distributed separately or included in software packages
   
outside this repository."

The libxenvchan.h header file is an Lesser GPLv2.1. Which would
imply that one could ignore the giant @section LICENSE in the libxenvchan.h
and treat it as non-GPL...

But to make the matters more complicated the ./COPYING files continues
with:

"Instead we specify a much more relaxed
BSD-style license. Affected files include the Xen interface headers 
   
(xen/include/public/COPYING), MiniOS (extras/mini-os) and various   
   
drivers, support functions and header files within Xen-aware Linux  
   
source trees.  In all such cases, license terms are stated at the top   
   
of the file or in a COPYING file in the same directory. Note that   
   
_any_ file that is modified and then distributed within a Linux kernel  
   
is still subject to the GNU GPL.  "

Great, so if I break this up:

 a) "Xen interface headers (xen/include/public/COPYING),"
 b) "MiniOS (..) and various drivers,"
 c) "support functions and header files with Xen-aware Linux
 source trees."

Well, libxenvchan is a).

What?

Yes, if I look in xen/include/public/COPYING it says:

"
XEN NOTICE
==

This copyright applies to all files within this subdirectory and its
subdirectories:
  include/public/*.h
  include/public/hvm/*.h
  include/public/io/*.h

The intention is that these files can be freely copied into the source
tree of an operating system when porting that OS to run on Xen. Doing
so does *not* cause the OS to become subject to the terms of the GPL
"

and libxenvchan.h is in ./xen/include/public/io/libxenvchan.h

Also it is in FreeBSD (source tree): ./sys/xen/interface/io/libxenvchan.h

That looks to be an oversigh. The commit that introduced the file is:
commit 1a16a3351ff2f2cf9f0cc0a27c89a0652eb8dfb4
Author: Daniel De Graaf 
Date:   Thu Oct 6 19:44:40 2011 +0100

libvchan: interdomain communications library

As such I was wondering if the folks who wrote/checked the file in
could weight in on what their intention was in regards to this file?

Thanks!

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

Re: [Xen-devel] identify a Xen PV domU to fix devmem_is_allowed

[Olaf Hering]

On Tue, Mar 22, Boris Ostrovsky wrote:

> Can you apply PAT changes from
> http://lists.xenproject.org/archives/html/xen-devel/2016-03/msg02127.html
> and see if it helps?
> 
> It should at least get rid of the splat (patch 3 is the one addresses
> no-MTRR problem that I mentioned above). We should still fix
> devmem_is_allowed() though.

Boris, using 02f037d^..b6350c2 fixes biosdevname in dom0 and reading
/dev/mem in a PV domU. Thanks.

Olaf

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

Re: [Xen-devel] XSA-180 follow-up: repurpose xenconsoled for logging

[Doug Goldstein]

On 6/6/16 5:12 AM, George Dunlap wrote:
> On 03/06/16 18:38, Andrew Cooper wrote:
>> On 01/06/16 15:00, Wei Liu wrote:
>>> Hi all
>>>



> FWIW, the libvirt project has exactly the same problem, and they did the
> analog of what Wei is proposing -- they added a new daemon, virtlogd, to
> handle all the console and debug log rotation in a fashion resistant to
> DoSing.  Without reading their discussion, it's reasonable to assume
> that using system logging was at least considered using system-level
> logging before deciding to write their own code.

If I recall they use RPCs and the logs are generated as a best effort to
not block QEMU.

> 
> We already have a daemon to do logging of consoles; it just doesn't have
> any of the logrotate features that are needed to make it robust against
> DoS.  There's no sense in having log rotation code in two places, so
> upgrading xenconsoled to do what virtlogd is doing makes more sense than
> say, either writing our own, or stealing virtlogd.

What if we made xl / libxl really good at the limited scope of things it
should be good at and left the other bits to others. At this point it
seems like yet another feature that xl / libxl is gaining that matches
what libvirt does. Maybe an approach is something you appear to suggest
and just point people to virtlogd and ask the libvirt guys if they would
make it a separate package. Honestly it seems like xl could slim down
from a feature set perspective and focus on improving libxl / libvirt
interaction. That's something that the Xen community has been interested
in to better support OpenStack anyway.

Just my 2 cents.

-- 
Doug Goldstein



signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 0/6] xenconsoled: rotating log file abstration

[Doug Goldstein]

On 6/6/16 10:59 AM, Wei Liu wrote:
> Wei Liu (6):
>   xenconsoled: introduce log file abstraction
>   xenconsoled: switch hypervisor log to use logfile abstraction
>   xenconsoled: switch guest log to use logfile abstraction
>   xenconsoled: delete two now unused functions
>   xenconsoled: options to control log rotation
>   xenconsoled: handle --log-backups 0 in logfile_rollover
> 
>  tools/console/daemon/io.c  | 139 +++
>  tools/console/daemon/logfile.c | 243 
> +
>  tools/console/daemon/logfile.h |  57 ++
>  tools/console/daemon/main.c|  13 ++-
>  4 files changed, 383 insertions(+), 69 deletions(-)
>  create mode 100644 tools/console/daemon/logfile.c
>  create mode 100644 tools/console/daemon/logfile.h
> 

Is there a mechanism for me to turn this off entirely? Due to some very
real possibilities like logging overwhelming xenconsoled and causing
qemu to block this is just not something that I will be able to roll out
in any of my deployments. We would rather lose logs than block qemu.

-- 
Doug Goldstein



signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] arm/acpi: Add Server Base System Architecture UART support

[Julien Grall]

On 06/06/2016 20:55, Shanker Donthineni wrote:

On 06/06/2016 11:04 AM, Julien Grall wrote:

On 03/06/16 18:39, Shanker Donthineni wrote:

The ARM Server Base System Architecture describes a generic UART
interface. It doesn't support clock control registers, modem
control, DMA and hardware flow control features. So, extend the
driver probe() to handle SBSA interface and skip the accessing
PL011 registers that are not described in SBSA document
(ARM-DEN-0029 Version 3.0, 6 APPENDIX B: GENERIC UART).

Signed-off-by: Shanker Donthineni 
---
Changes since v2:
Edited commit text to include SBSA document version.
Remove setting baudrate code completely as per Julien's suggestion.


This item and ...


Support both the SBSA interface types ACPI_DBG2_SBSA &

ACPI_DBG2_SBSA_32.

Replace MIS references with combination of RIS & IMSC.


this one need some description in the commit message to explain why it is fine 
for non-SBSA UART to do it.


I have not tested this patch on non-SBSA UART platform, let me apply this 
change only for SBSA UART driver
for safe side.


This patch could be easily tested on the foundation model. I don't want 
to see "if SBSA UART" in the interrupt handler. The code there should be 
straight-forward.


What I am asking for is explaining in a commit message why MIS could be 
replaced by RIS & IMSC. You must have read the doc to find that.


A comment in the code to say this necessary to be SBSA compliant v2.x 
would be good to avoid change reverting back in the future to MIS.


Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v3] arm/acpi: Add Server Base System Architecture UART support

[Shanker Donthineni]

Hi Julien,

On 06/06/2016 11:04 AM, Julien Grall wrote:
> Hi Shanker,
>
> On 03/06/16 18:39, Shanker Donthineni wrote:
>> The ARM Server Base System Architecture describes a generic UART
>> interface. It doesn't support clock control registers, modem
>> control, DMA and hardware flow control features. So, extend the
>> driver probe() to handle SBSA interface and skip the accessing
>> PL011 registers that are not described in SBSA document
>> (ARM-DEN-0029 Version 3.0, 6 APPENDIX B: GENERIC UART).
>>
>> Signed-off-by: Shanker Donthineni 
>> ---
>> Changes since v2:
>> Edited commit text to include SBSA document version.
>> Remove setting baudrate code completely as per Julien's suggestion.
>
> This item and ...
>
>> Support both the SBSA interface types ACPI_DBG2_SBSA &
> ACPI_DBG2_SBSA_32.
>> Replace MIS references with combination of RIS & IMSC.
>
> this one need some description in the commit message to explain why it is 
> fine for non-SBSA UART to do it.
>
I have not tested this patch on non-SBSA UART platform, let me apply this 
change only for SBSA UART driver
for safe side.

> The later also needs some explanation why MIS could be replaced by RIS & IMSC.
>
> Lastly, IHMO those 2 items should be in separate patch to make the review 
> easier.
>
>>
>> Changes since v1:
>> Don't access UART registers that are not part of SBSA document.
>> Move setting baudrate function to a separate function.
>>
>>   xen/drivers/char/pl011.c | 76
> ++--
>>   1 file changed, 41 insertions(+), 35 deletions(-)
>>
>> diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
>> index 1212d5c..3497d61 100644
>> --- a/xen/drivers/char/pl011.c
>> +++ b/xen/drivers/char/pl011.c
>> @@ -31,7 +31,7 @@
>>   #include 
>>
>>   static struct pl011 {
>> -unsigned int baud, clock_hz, data_bits, parity, stop_bits;
>> +unsigned int data_bits, parity, stop_bits;
>>   unsigned int irq;
>>   void __iomem *regs;
>>   /* UART with IRQ line: interrupt-driven I/O. */
>> @@ -41,6 +41,7 @@ static struct pl011 {
>>   /* struct timer timer; */
>>   /* unsigned int timeout_ms; */
>>   /* bool_t probing, intr_works; */
>> +bool sbsa;  /* ARM SBSA generic interface */
>>   } pl011_com = {0};
>>
>>   /* These parity settings can be ORed directly into the LCR. */
>> @@ -57,7 +58,9 @@ static void pl011_interrupt(int irq, void *data,
> struct cpu_user_regs *regs)
>>   {
>>   struct serial_port *port = data;
>>   struct pl011 *uart = port->uart;
>> -unsigned int status = pl011_read(uart, MIS);
>> +unsigned int status = pl011_read(uart, RIS);
>> +
>> +status &= pl011_read(uart, IMSC);
>>
>>   if ( status )
>>   {
>> @@ -76,7 +79,7 @@ static void pl011_interrupt(int irq, void *data,
> struct cpu_user_regs *regs)
>>   if ( status & (TXI) )
>>   serial_tx_interrupt(port, regs);
>>
>> -status = pl011_read(uart, MIS);
>> +status = pl011_read(uart, RIS) & pl011_read(uart, IMSC);
>
> Please introduce a helper to read the status.
>
>>   } while (status != 0);
>>   }
>>   }
>
> Regards,
>

-- 
Shanker Donthineni
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux 
Foundation Collaborative Project


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

[Xen-devel] [xen-unstable test] 95309: tolerable FAIL

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-xl-credit2 15 guest-start/debian.repeat fail in 95281 pass in 
95309
 test-armhf-armhf-xl  15 guest-start/debian.repeat   fail pass in 95281
 test-amd64-amd64-xl-rtds  6 xen-bootfail pass in 95281

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-rtds  9 debian-installfail in 95281 like 95261
 build-amd64-rumpuserxen   6 xen-buildfail   like 95281
 build-i386-rumpuserxen6 xen-buildfail   like 95281
 test-amd64-i386-freebsd10-amd64 10 guest-start fail like 95281
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 95281
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 95281
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail like 95281
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 95281

Tests which did not succeed, but are not blocking:
 test-amd64-i386-rumpuserxen-i386  1 build-check(1)   blocked  n/a
 test-amd64-amd64-rumpuserxen-amd64  1 build-check(1)   blocked n/a
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass

version targeted for testing:
 xen  c2a17869d5dcd845d646bf4db122cad73596a2be
baseline version:
 xen  c2a17869d5dcd845d646bf4db122cad73596a2be

Last test of basis95309  2016-06-06 01:59:13 Z0 days
Testing same since0  1970-01-01 00:00:00 Z 16958 days0 attempts

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

Re: [Xen-devel] [RFC for-4.8 v2 7/7] xen/arm: Map mmio-sram nodes as normal un-cached rwx memory

[Julien Grall]

Hi Edgar,

On 03/06/16 14:29, Edgar E. Iglesias wrote:

From: "Edgar E. Iglesias" 

Map mmio-sram nodes as normal un-cached MEMORY with RWX perms.
If the node has set the no-memory-wc property, we map it as
DEVICE RW.


Please add a path/link to the bindings in the commit message.



Signed-off-by: Edgar E. Iglesias 
---
  xen/arch/arm/domain_build.c | 37 +
  1 file changed, 37 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 064feb3..2a2316b 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -54,6 +54,32 @@ struct map_range_data
  const struct map_attr *attr;
  };

+static const struct map_attr mattr_device_rw __initconst =
+{
+.memattr = MATTR_DEV,
+.access = p2m_access_rw,
+};
+
+static const struct map_attr mattr_memory_nc_rwx __initconst =
+{
+.memattr = MATTR_MEM_NC,
+.access = p2m_access_rwx,
+};
+
+static const struct dt_device_match dev_map_attrs[] __initconst =
+{
+{
+__DT_MATCH_COMPATIBLE("mmio-sram"),
+__DT_MATCH_PROPS("no-memory-wc"),
+.data = _device_rw,
+},
+{
+__DT_MATCH_COMPATIBLE("mmio-sram"),
+.data = _memory_nc_rwx,
+},
+{ /* sentinel */ },
+};
+
  //#define DEBUG_DT

  #ifdef DEBUG_DT
@@ -1201,6 +1227,16 @@ static int handle_device(struct domain *d, struct 
dt_device_node *dev,
  return 0;
  }

+static const struct map_attr *lookup_map_attr(struct dt_device_node *node,
+const struct map_attr *parent_attr)


The indentation looks wrong here.


+{
+const struct dt_device_match *r;
+
+/* Search and if nothing matches, use the parent's attributes.  */
+r = dt_match_node(dev_map_attrs, node);


Newline here please.


+return r ? r->data : parent_attr;


I would explain this behavior in the commit message and give some 
rationale why it is fine to do that.



+}
+
  static int handle_node(struct domain *d, struct kernel_info *kinfo,
 struct dt_device_node *node,
 const struct map_attr *attr)
@@ -1290,6 +1326,7 @@ static int handle_node(struct domain *d, struct 
kernel_info *kinfo,
 "WARNING: Path %s is reserved, skip the node as we may re-use the 
path.\n",
 path);

+attr = lookup_map_attr(node, attr);
  res = handle_device(d, node, attr);
  if ( res)
  return res;



Regards,

--
Julien Grall

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

Re: [Xen-devel] [RFC for-4.8 v2 6/7] xen/device-tree: Add an mmio-sram bus

[Julien Grall]

Hi Edgar,

On 03/06/16 14:29, Edgar E. Iglesias wrote:

From: "Edgar E. Iglesias" 

Add an mmio-sram bus that prevents sram sub areas from
being re-mapped. These sub-areas describe allocations and
not mappings.


mmio-sram is not a bus and the region below should point to a valid 
physical address.


So why do you want that?

Regards,

--
Julien Grall

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

Re: [Xen-devel] [RFC for-4.8 v2 5/7] xen/arm: domain_build: Plumb for different mapping attributes

[Julien Grall]

Hi Edgar,

On 03/06/16 14:29, Edgar E. Iglesias wrote:

From: "Edgar E. Iglesias" 

Add plumbing for passing around mapping attributes. This
is in preparation to allow us to differentiate the attributes
for specific device nodes.

We still use the same DEVICE mappings for all nodes so this
patch has no functional change.

Signed-off-by: Edgar E. Iglesias 
---
  xen/arch/arm/domain_build.c | 57 +
  1 file changed, 42 insertions(+), 15 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index e4fed4b..064feb3 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -42,6 +42,18 @@ static void __init parse_dom0_mem(const char *s)
  }
  custom_param("dom0_mem", parse_dom0_mem);

+struct map_attr
+{
+unsigned int memattr;
+p2m_access_t access;


Based on my comment on patch #2, access should not be used for other 
purpose than memaccess. So ...



+};
+
+struct map_range_data
+{
+struct domain *d;
+const struct map_attr *attr;


.. this could be replaced by "unsigned int memattr" directly.


+};
+
  //#define DEBUG_DT

  #ifdef DEBUG_DT


[...]


@@ -1322,6 +1344,11 @@ static int handle_node(struct domain *d, struct 
kernel_info *kinfo,

  static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
  {
+const struct map_attr default_attr =
+{
+.memattr = MATTR_DEV,
+.access = d->arch.p2m.default_access,
+};


Please add a comment to explain that by default, region are always 
mapped the most restrictive way with device attribute.



  const void *fdt;
  int new_size;
  int ret;
@@ -1341,7 +1368,7 @@ static int prepare_dtb(struct domain *d, struct 
kernel_info *kinfo)

  fdt_finish_reservemap(kinfo->fdt);

-ret = handle_node(d, kinfo, dt_host);
+ret = handle_node(d, kinfo, dt_host, _attr);
  if ( ret )
  goto err;




Regards,

--
Julien Grall

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

Re: [Xen-devel] [RFC for-4.8 v2 2/7] xen/arm: Rename and generalize un/map_regions_rw_cache

[Julien Grall]

Hi Edgar,

On 03/06/16 14:29, Edgar E. Iglesias wrote:

From: "Edgar E. Iglesias" 

Rename and generalize un/map_regions_rw_cache into
un/map_regions.


I would name it map_regions_mattr (or something similar) to stop people 
using this helper to map real RAM.



The new functions take the mapping
attributes and access permissions as arguments.


I overlooked it when I reviewed {,un}map_regions_rw_cache. p2m_access_t 
is for memaccess to restrict the permission. The helper should always 
use d->arch.p2m.default_access.


Can you fix it in a separate patch and request for backport to Xen 4.7?

Regards,

--
Julien Grall

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

Re: [Xen-devel] Xen, systemd, and selinux

[M A Young]

On Mon, 6 Jun 2016, George Dunlap wrote:

> Do you know where the "upstream" for these rules are, and how to get
> them changed in a way that will trickle down eventually to CentOS?

I got it fixed in very recent selinux-policy packages (see 
https://bugzilla.redhat.com/show_bug.cgi?id=1334115 ) on Fedora. The only 
automatic trickle down from Fedora to RHEL to CentOS that I know of is to 
later releases so it will presumably be fixed in CentOS 8. I think there 
is a greater chance of it being backported to earlier versions if it is 
reported as a bug against whichever versions of RHEL you want it for on 
bugzilla or via a support contract if you have one.

Michael Young

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

Re: [Xen-devel] [RFC for-4.8 v2 4/7] xen/device-tree: Make dt_match_node match props

[Julien Grall]

Hi Edgar,

On 03/06/16 14:29, Edgar E. Iglesias wrote:

From: "Edgar E. Iglesias" 

Make dt_match_node match for existing properties.
We only search for the existance of the properties, not


s/existance/existence/


for specific values.


[..]


diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
index b348913..f13d186 100644
--- a/xen/include/xen/device_tree.h
+++ b/xen/include/xen/device_tree.h
@@ -30,6 +30,7 @@ struct dt_device_match {
  const char *type;
  const char *compatible;
  const bool_t not_available;
+const char **props;


I would add a comment above the field to explain the behavior.


  const void *data;
  };

@@ -37,11 +38,13 @@ struct dt_device_match {
  #define __DT_MATCH_TYPE(typ).type = typ
  #define __DT_MATCH_COMPATIBLE(compat)   .compatible = compat
  #define __DT_MATCH_NOT_AVAILABLE()  .not_available = 1
+#define __DT_MATCH_PROPS(p...)  .props = (const char *[]) { p, NULL }


Why the cast?



  #define DT_MATCH_PATH(p){ __DT_MATCH_PATH(p) }
  #define DT_MATCH_TYPE(typ)  { __DT_MATCH_TYPE(typ) }
  #define DT_MATCH_COMPATIBLE(compat) { __DT_MATCH_COMPATIBLE(compat) }
  #define DT_MATCH_NOT_AVAILABLE(){ __DT_MATCH_NOT_AVAILABLE() }
+#define DT_MATCH_PROPS(p...){ __DT_MATCH_PROPS(p) }

  typedef u32 dt_phandle;


Regards,

--
Julien Grall

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

Re: [Xen-devel] [RFC for-4.8 v2 3/7] xen/device-tree: Add __DT_MATCH macros without braces

[Julien Grall]

Hi Edgar,

On 03/06/16 14:29, Edgar E. Iglesias wrote:

From: "Edgar E. Iglesias" 

Add __DT_MATCH macros without braces to allow the creation
of match descriptors with multiple combined match options.

Signed-off-by: Edgar E. Iglesias 


Acked-by: Julien Grall 

Regards,

--
Julien Grall

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

Re: [Xen-devel] "xl vcpu-set" not persistent across reboot?

[Andrew Cooper]

On 06/06/16 18:20, Wei Liu wrote:
> Use Stefano's new email address
>
> On Mon, Jun 06, 2016 at 06:18:06PM +0100, Wei Liu wrote:
>> On Fri, Jun 03, 2016 at 05:35:20PM +0100, Wei Liu wrote:
>>> On Fri, Jun 03, 2016 at 08:42:11AM -0600, Jan Beulich wrote:
>>> On 03.06.16 at 15:41,  wrote:
> On Fri, Jun 03, 2016 at 02:29:12AM -0600, Jan Beulich wrote:
>> Ian, Wei,
>>
>> is it intentional that rebooting a (HVM) guest after having altered its
>> vCPU count will reset it back to the vCPU count it was originally
>> started with? That doesn't seem very natural - if one hotplugs a CPU
>> into a physical system and then reboots, that CPU will remain there.
>>
> This is probably an oversight.
>
> I've added this to my list of things to look at after the release.
 Thanks!

>>> I got a patch ready.  But QEMU upstream refuses to start on the receiving 
>>> end
>>> with following error message:
>>>
>>> qemu-system-i386: Unknown savevm section or instance 'cpu_common' 1
>>> qemu-system-i386: load of migration failed: Invalid argument
>>>
>>> With QEMU traditional HVM guest and PV guest, the guest works fine -- up
>>> and running with all hot plugged cpus available.
>>>
>>> So I think the relevant libxl information is transmitted but we also
>>> need to fix QEMU upstream. But that's a separate issue.
>>>
>>> Wei.
>>>
>>> ---8<---
>>> From 790ff77c6307b341dec0b4cc5e2d394e42f82e7c Mon Sep 17 00:00:00 2001
>>> From: Wei Liu 
>>> Date: Fri, 3 Jun 2016 16:38:32 +0100
>>> Subject: [PATCH] libxl: update vcpus bitmap in retrieved geust config
>>>
>>> ... because the available vcpu bitmap can change during domain life time
>>> due to cpu hotplug and unplug.
>>>
>>> Reported-by: Jan Beulich 
>>> Signed-off-by: Wei Liu 
>> This patch has two issues:
>> 1. The code to allocate bitmap is wrong
>> 2. The check to see if vcpu is available is wrong
>>
>> It happens to work on PV and qemu-trad because they don't really rely
>> on the bitmap provided.
>>
>> #1 is fixed.
>>
>> For #2, I haven't really gotten to the point today on how to correctly
>> get the number of online vcpus.
>>
>> The current issue I discover is that:
>>
>>   xl vcpu-set jessie-hvm 4
>>   xl list -l jessie-hvm | less # search for avail_vcpus
>>
>> A vcpu is not really considered online from xen's point of view, unless
>> the guest explicitly activates it, like in guest `echo 1 >
>> .../cpu1/online`.
>>
>> This is still not desirable because it would still cause qemu upstream
>> migration to fail. I will see if there is other way to figure out how
>> many vcpus are there.
>>
>> Off the top of my head I might need to interrogate QEMU for that. I will
>> continue investigation later.
>>
>> Any hint on how to effectively identify online vcpus is very welcomed.

Why does qemu even care?  It has nothing to do with vcpu handling. 
There should not be any qemu vcpu records in the first place.

~Andrew

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

Re: [Xen-devel] [PATCH RFC 19/20] acpi: Set HW_REDUCED_ACPI in FADT if IOAPIC is not supported

[Boris Ostrovsky]

On 06/06/2016 09:38 AM, Jan Beulich wrote:
 On 06.04.16 at 03:25,  wrote:
>> With this flags set guests will not try to set up SCI.
> I've just read through the respective ACPI spec section again, and
> I couldn't find a reference to SCI from there ("Hardware-Reduced
> ACPI"). Can you clarify this connection please. Also there are other
> consequences of setting that flag, so in order to understand the
> reasons behind this change in case of future problems I think the
> description here will need to be significantly extended, despite the
> change being so small.

My understanding is that hardware-reduced platforms don't use ACPI
Platform Event Model (Sec. 4.1.1) and that model requires SCI (and vice
versa --- SCI is present when ACPI Platform Event Model is in use). The
(somewhat indirect) evidence of this is in section 4.6 "The ACPI
Hardware Model" where is says: "In the ACPI Legacy state, the ACPI event
model is disabled (no SCIs are generated) ..."

-boris

>
>> --- a/xen/common/libacpi/build.c
>> +++ b/xen/common/libacpi/build.c
>> @@ -555,6 +555,8 @@ void acpi_build_tables(struct acpi_config *config, 
>> unsigned long physical)
>>  fadt->x_dsdt = config->mem_ops.v2p(dsdt);
>>  fadt->firmware_ctrl   = config->mem_ops.v2p(facs);
>>  fadt->x_firmware_ctrl = config->mem_ops.v2p(facs);
>> +if ( !(config->table_flags & ACPI_BUILD_IOAPIC) )
>> +fadt->flags |= (1<<20); /* HW_REDUCED_ACPI */
> I think this would better be made a suitable #define in acpi2_0.h.
>
> Jan
>



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

Re: [Xen-devel] [PATCH v5 5/9] monitor: ARM SMC events

[Tamas K Lengyel]

On Mon, Jun 6, 2016 at 10:38 AM, Julien Grall  wrote:
> Hi Tamas,
>
> On 06/06/16 17:14, Tamas K Lengyel wrote:
>>
>> On Mon, Jun 6, 2016 at 9:56 AM, Tamas K Lengyel 
>> wrote:
>>>
>>> On Mon, Jun 6, 2016 at 9:54 AM, Julien Grall 
>>> wrote:
>>
>> So either way, I don't see a technical reason why Xen should silently
>> swallow any SMC trap if the vm_event user specifically asked them to
>> be forwarded. Other then it being odd that some ARM chips have varying
>> behavior regarding a subset of SMC instructions, it should not affect
>> when the vm_event user gets the events. If the user requests that it
>> wants to get notified any time an SMC is trapped to the VMM, it
>> should, regardless of whether that makes sense for "us". Depending on
>> the use-case of the user, indeed it may need extra information if it
>> wants to do emulation. If that need arises, the interface can easily
>> be extended to accommodate that usecase. We can also add a comment
>> saying that the forwarded events may also include ones with failed
>> condition checks depending on the CPU implementation. Also, it would
>> also be possible in the future to add a monitor configuration bit
>> where the user can specify if it wants the failed condition check SMCs
>> ignored by default or not. At this time however I want to start simple
>> and just forward all events, adding more bits and pieces only as
>> needed.
>
>
> We disagree on what is a "starting simple". It easier to relax than
> restricting a behavior later one.
>
> Even if we decide to add a bit to ignore some SMC in a later version of Xen,
> the introspection app will need to carry the burden mentioned in lengthly
> way on the previous mails because they may want to support older version of
> Xen.
>
> It would not be that difficult to provide a clean interface from beginning,
> which would allow to support more than your usecase.
>
> Anyway, I am not gonna ack this patch for the modification in
> arch/arm/traps.c because I don't think this is the right way to go. I will
> let Stefano deciding on this one.
>

Sounds good to me, my use-case is fine either way so ultimately it's
up to you guys.

Tamas

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

Re: [Xen-devel] Is: 'basic pci bridge and root device support. 'Was:Re: Discussion about virtual iommu support for Xen guest

[Konrad Rzeszutek Wilk]

On Mon, Jun 06, 2016 at 03:55:06AM -0600, Jan Beulich wrote:
> >>> On 03.06.16 at 21:51,  wrote:
> >>  For HVMLite, there is specifically no qemu, and we need something which
> >> can function when we want PCI Passthrough to work.  I am quite confident
> >> that the correct solution here is to have a basic host bridge/root port
> >> implementation in Xen (as we already have 80% of this already), at which
> >> point we don't need any qemu interaction for PCI Passthough at all, even
> >> for HVM guests.
> > 
> > Could you expand on this a bit?
> > 
> > I am asking b/c some time ago I wrote in Xen code to construct a full view
> > of the bridges->devices (and various in branching) so that I could renumber
> > the bus values and its devices (expand them) on bridges. This was solely 
> > done
> > so that I could use SR-IOV devices on non-SR-IOV capable BIOSes.
> > 
> > I am wondering how much of the basic functionality (enumeration, keeping
> > track, etc) could be worked in this 'basic host bridge/root port' 
> > implementation
> > idea of yours.
> 
> Keep in mind that your work was for the host's PCI, while here
> we're talking about the guest's.

Right, but some of this accounting code could be added in Xen to help
with keeping track of physical vs virtual layouts.

> 
> Jan
> 

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

Re: [Xen-devel] [RFC for-4.8 v2 1/7] xen/arm: Add MATTR_MEM_NC for normal non-cacheable memory

[Julien Grall]

Hi Edgar,

On 03/06/16 14:29, Edgar E. Iglesias wrote:

From: "Edgar E. Iglesias" 

Add the MATTR_MEM_NC macro describing normal non-cacheable memory.

Signed-off-by: Edgar E. Iglesias 
---
  xen/arch/arm/p2m.c | 1 +
  xen/include/asm-arm/page.h | 1 +
  2 files changed, 2 insertions(+)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 838d004..0f1e1b1 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -343,6 +343,7 @@ static lpae_t mfn_to_p2m_entry(unsigned long mfn, unsigned 
int mattr,
  e.p2m.sh = LPAE_SH_INNER;
  break;

+case MATTR_MEM_NC:


I would add a comment here to explain why non-cacheable memory should 
always be outer-shareable (see DDI 0406C.b B3-1376 to 1377).


For ideas on what to write, see mfn_to_xen_entry in include/asm-arm/page.h


  case MATTR_DEV:
  e.p2m.sh = LPAE_SH_OUTER;
  break;
diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
index a94e978..45d2e2c 100644
--- a/xen/include/asm-arm/page.h
+++ b/xen/include/asm-arm/page.h
@@ -73,6 +73,7 @@
   *
   */
  #define MATTR_DEV 0x1
+#define MATTR_MEM_NC  0x5
  #define MATTR_MEM 0xf

  /* Flags for get_page_from_gva, gvirt_to_maddr etc */



Regards,

--
Julien Grall

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

Re: [Xen-devel] "xl vcpu-set" not persistent across reboot?

[Wei Liu]

Use Stefano's new email address

On Mon, Jun 06, 2016 at 06:18:06PM +0100, Wei Liu wrote:
> On Fri, Jun 03, 2016 at 05:35:20PM +0100, Wei Liu wrote:
> > On Fri, Jun 03, 2016 at 08:42:11AM -0600, Jan Beulich wrote:
> > > >>> On 03.06.16 at 15:41,  wrote:
> > > > On Fri, Jun 03, 2016 at 02:29:12AM -0600, Jan Beulich wrote:
> > > >> Ian, Wei,
> > > >> 
> > > >> is it intentional that rebooting a (HVM) guest after having altered its
> > > >> vCPU count will reset it back to the vCPU count it was originally
> > > >> started with? That doesn't seem very natural - if one hotplugs a CPU
> > > >> into a physical system and then reboots, that CPU will remain there.
> > > >> 
> > > > 
> > > > This is probably an oversight.
> > > > 
> > > > I've added this to my list of things to look at after the release.
> > > 
> > > Thanks!
> > > 
> > 
> > I got a patch ready.  But QEMU upstream refuses to start on the receiving 
> > end
> > with following error message:
> > 
> > qemu-system-i386: Unknown savevm section or instance 'cpu_common' 1
> > qemu-system-i386: load of migration failed: Invalid argument
> > 
> > With QEMU traditional HVM guest and PV guest, the guest works fine -- up
> > and running with all hot plugged cpus available.
> > 
> > So I think the relevant libxl information is transmitted but we also
> > need to fix QEMU upstream. But that's a separate issue.
> > 
> > Wei.
> > 
> > ---8<---
> > From 790ff77c6307b341dec0b4cc5e2d394e42f82e7c Mon Sep 17 00:00:00 2001
> > From: Wei Liu 
> > Date: Fri, 3 Jun 2016 16:38:32 +0100
> > Subject: [PATCH] libxl: update vcpus bitmap in retrieved geust config
> > 
> > ... because the available vcpu bitmap can change during domain life time
> > due to cpu hotplug and unplug.
> > 
> > Reported-by: Jan Beulich 
> > Signed-off-by: Wei Liu 
> 
> This patch has two issues:
> 1. The code to allocate bitmap is wrong
> 2. The check to see if vcpu is available is wrong
> 
> It happens to work on PV and qemu-trad because they don't really rely
> on the bitmap provided.
> 
> #1 is fixed.
> 
> For #2, I haven't really gotten to the point today on how to correctly
> get the number of online vcpus.
> 
> The current issue I discover is that:
> 
>   xl vcpu-set jessie-hvm 4
>   xl list -l jessie-hvm | less # search for avail_vcpus
> 
> A vcpu is not really considered online from xen's point of view, unless
> the guest explicitly activates it, like in guest `echo 1 >
> .../cpu1/online`.
> 
> This is still not desirable because it would still cause qemu upstream
> migration to fail. I will see if there is other way to figure out how
> many vcpus are there.
> 
> Off the top of my head I might need to interrogate QEMU for that. I will
> continue investigation later.
> 
> Any hint on how to effectively identify online vcpus is very welcomed.
> 
> Wei.

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

Re: [Xen-devel] "xl vcpu-set" not persistent across reboot?

[Wei Liu]

On Fri, Jun 03, 2016 at 05:35:20PM +0100, Wei Liu wrote:
> On Fri, Jun 03, 2016 at 08:42:11AM -0600, Jan Beulich wrote:
> > >>> On 03.06.16 at 15:41,  wrote:
> > > On Fri, Jun 03, 2016 at 02:29:12AM -0600, Jan Beulich wrote:
> > >> Ian, Wei,
> > >> 
> > >> is it intentional that rebooting a (HVM) guest after having altered its
> > >> vCPU count will reset it back to the vCPU count it was originally
> > >> started with? That doesn't seem very natural - if one hotplugs a CPU
> > >> into a physical system and then reboots, that CPU will remain there.
> > >> 
> > > 
> > > This is probably an oversight.
> > > 
> > > I've added this to my list of things to look at after the release.
> > 
> > Thanks!
> > 
> 
> I got a patch ready.  But QEMU upstream refuses to start on the receiving end
> with following error message:
> 
> qemu-system-i386: Unknown savevm section or instance 'cpu_common' 1
> qemu-system-i386: load of migration failed: Invalid argument
> 
> With QEMU traditional HVM guest and PV guest, the guest works fine -- up
> and running with all hot plugged cpus available.
> 
> So I think the relevant libxl information is transmitted but we also
> need to fix QEMU upstream. But that's a separate issue.
> 
> Wei.
> 
> ---8<---
> From 790ff77c6307b341dec0b4cc5e2d394e42f82e7c Mon Sep 17 00:00:00 2001
> From: Wei Liu 
> Date: Fri, 3 Jun 2016 16:38:32 +0100
> Subject: [PATCH] libxl: update vcpus bitmap in retrieved geust config
> 
> ... because the available vcpu bitmap can change during domain life time
> due to cpu hotplug and unplug.
> 
> Reported-by: Jan Beulich 
> Signed-off-by: Wei Liu 

This patch has two issues:
1. The code to allocate bitmap is wrong
2. The check to see if vcpu is available is wrong

It happens to work on PV and qemu-trad because they don't really rely
on the bitmap provided.

#1 is fixed.

For #2, I haven't really gotten to the point today on how to correctly
get the number of online vcpus.

The current issue I discover is that:

  xl vcpu-set jessie-hvm 4
  xl list -l jessie-hvm | less # search for avail_vcpus

A vcpu is not really considered online from xen's point of view, unless
the guest explicitly activates it, like in guest `echo 1 >
.../cpu1/online`.

This is still not desirable because it would still cause qemu upstream
migration to fail. I will see if there is other way to figure out how
many vcpus are there.

Off the top of my head I might need to interrogate QEMU for that. I will
continue investigation later.

Any hint on how to effectively identify online vcpus is very welcomed.

Wei.

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

Re: [Xen-devel] [RFC] xen/arm: build: add missed dependency for head.S

[Julien Grall]

(CC Ian)

Hi,

On 03/06/16 11:07, Wei Chen wrote:

When we update the header files that had been included in head.S.
The build system would not re-compile the head.S. Because in the
build rules, the dependencies are setting to .*.d (eg. DEPS = .*.d)
files in the same folder as Makefile.

But head.S is very special, it was used by the Makefile in the parent
folder: "ALL_OBJS := $(TARGET_SUBARCH)/head.o".

In this case, the build system could not find the dependency in DEPS.
When we update the header files, the build system is unware of this
update. If we re-build the Xen without doing make clean or touching
the head.S, the build system will not recompile the head.S.

Signed-off-by: Wei Chen 
---
In my mind, the better way to fix this bug is converting the DEPS from
ALL_OBJS. But I am afraid of the impact. I am not sure whether there
are some dependencies are not generated from obj files.


My knowledge of the build system is limited. Stefano, Ian, Wei, could 
you give a look?


Regards,


---
xen/Rules.mk  | 3 ++-
  xen/arch/arm/Makefile | 2 ++
  2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index 961d533..1247b61 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -92,9 +92,10 @@ LDFLAGS += $(LDFLAGS-y)

  include $(BASEDIR)/arch/$(TARGET_ARCH)/Rules.mk

+DEPS = .*.d
+
  include Makefile

-DEPS = .*.d
  define gendep
  ifneq ($(1),$(subst /,:,$(1)))
  DEPS += $(dir $(1)).$(notdir $(1)).d
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index af4d0e1..9e38da3 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -51,6 +51,8 @@ endif

  ALL_OBJS := $(TARGET_SUBARCH)/head.o $(ALL_OBJS)

+DEPS += $(TARGET_SUBARCH)/.head.o.d
+
  $(TARGET): $(TARGET)-syms $(TARGET).axf
$(OBJCOPY) -O binary -S $< $@
  ifeq ($(CONFIG_ARM_64),y)



--
Julien Grall

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

Re: [Xen-devel] Xen, systemd, and selinux

[Konrad Rzeszutek Wilk]

On Mon, Jun 06, 2016 at 05:41:35PM +0100, George Dunlap wrote:
> Hey Michael,

CC-ing Olif and Daniel De Graaf,
> 
> Not sure if you know, I've been maintaining the Xen4CentOS packages; I
> suspect given the similarities between our systems we're solving the
> same issues; particularly with the systemd/selinux combination.
> 
> I've just ported my patchqueue up to 4.7-rc4, and it looks like the
> SELinux rules for xenstored -- at least the ones that come with CentOS
> 7 -- are outdated; they allow xenstored to open /proc/xen/privcmd
> (which is deprecated), but not /dev/xen/privcmd.
> 
> Do you know where the "upstream" for these rules are, and how to get
> them changed in a way that will trickle down eventually to CentOS?

You open a bug against selinux policies. For example see:
https://bugzilla.redhat.com/show_bug.cgi?id=1322625
https://bugzilla.redhat.com/show_bug.cgi?id=1334511
And (which is exactly what you are hitting):

Bug 1334115 - SELinux is preventing xenconsoled from 'ioctl' accesses on the 
chr_file /dev/xen/privcmd. (edit)

Since not all of them went in F24 one way you can work around
this is to have a new 'xen-tools-selinux' package that will
install the new SELinux policies.

However I have to confess I hadn't managed to fix the /dev/xen/privcmd.
It still pops up occasionaly. The fix 1334115 is:

dev/xen/blktap.*  -c  gen_context(system_u:object_r:xen_device_t,s0)
 /dev/xen/evtchn-c  
gen_context(system_u:object_r:xen_device_t,s0)
 /dev/xen/gntdev-c  
gen_context(system_u:object_r:xen_device_t,s0)
 /dev/xen/gntalloc  -c  gen_context(system_u:object_r:xen_device_t,s0)
+/dev/xen/privcmd   -c  gen_context(system_u:object_r:xen_device_t,s0)


Which I tried to replicate (see xen.fc)

For OL7 I did:

semanage fcontext -a -t xen_device_t /dev/xen/privcmd
restorecon -Rv /dev/xen/privcmd 

in the %post section of the spec file.

However oddly enough it does not always  work and I am not sure
what is up with that.

Also for OL7 I needed to do a bunch of other policies (see attached)
to get all of them SELinux complains out.

This is what I did in the %build:
   
%if 0%{?el7}
   
make -f /usr/share/selinux/devel/Makefile xenstored_policy.pp   
   
make -f /usr/share/selinux/devel/Makefile xenconsoled_policy.pp 
   
make -f /usr/share/selinux/devel/Makefile xen.fc
   
%endif  

and in %install:

%global modulenames xenstored_policy xenconsoled_policy 
# Usage: _format var format 
#   Expand 'modulenames' into various formats as needed 
#   Format must contain '$x' somewhere to do anything useful
%global _format() export %1=""; for x in %{modulenames}; do %1+=%2; %1+=" "; 
done;



%_format MODULES $x.pp  
install -d %{buildroot}%{_datadir}/selinux/packages 
install -m 0644 $MODULES %{buildroot}%{_datadir}/selinux/packages   
install -m 0644 xen.fc %{buildroot}%{_datadir}/selinux/packages   


And in the %post:

%post tools-selinux 
%_format MODULES %{_datadir}/selinux/packages/$x.pp 
%{_sbindir}/semodule -s %{selinuxtype} -i $MODULES  
%{_sbindir}/semanage fcontext -a -t xen_device_t /dev/xen/privcmd   
if %{_sbindir}/selinuxenabled ; then
%{_sbindir}/load_policy 
%{_sbindir}/restorecon -Rv /dev/xen/privcmd 
fi  
%endif 

CC-ing Olif.

> 
> As of 4.7-rc4, libxc will first try to open /dev/xen/privcmd, then
> *if* it fails with a certain set of error codes, it tries
> /proc/xen/privcmd instead.  Unfortunately, EACCES (the failure you get
> from SELinux denials) is not one of those error codes.  If you just
> add that error code in to the list of acceptable error codes, then
> things work for me.
> 
> Thanks,
>  -George
> 
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

module xenstored_policy 1.0;

require {
type xenstored_t;
type device_t;
type sysctl_fs_t;
type initrc_t;
class unix_stream_socket accept;
class dir search;
class 

Re: [Xen-devel] [PATCH RFC 18/20] libxc/acpi: Build ACPI tables for HVMlite guests

[Boris Ostrovsky]

On 06/06/2016 09:29 AM, Jan Beulich wrote:
 On 06.04.16 at 03:25,  wrote:
>> --- /dev/null
>> +++ b/tools/libxc/xc_acpi.c
>> @@ -0,0 +1,268 @@
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include 
>> +#include 
>> +#include 
> Why are these two needed here? The code here shouldn't care
> about the mode the guest may later want to run in.

These two are not needed.

>
>> +#include 
>> +#include 
>> +
>> +#include "xg_private.h"
>> +#include "xc_dom.h"
>> +#include "xenctrl.h"
>> +
>> +#include "acpi2_0.h"
>> +
>> +#define RESERVED_MEMORY_DYNAMIC_START 0xFC001000
>> +#define ACPI_PHYSICAL_ADDRESS 0x000EA020
>> +
>> +/* Initial allocation for ACPI tables */
>> +#define NUM_ACPI_PAGES  16
> With which other definitions do these three need to remain in sync?

NUM_ACPI_PAGES is private to this file.

ACPI_PHYSICAL_ADDRESS (RSDP pointer) needs to be between 0xe and 0xf, I 
picked this number because that's where most systems that I have appear to have 
it. (And by "most" I mean the two that I checked ;-))

RESERVED_MEMORY_DYNAMIC_START is one page after DSDT's SystemMemory (aka 
ACPI_INFO_PHYSICAL_ADDRESS). But then it looks like PVHv2 doesn't need 
SystemMemory so it can be anywhere (and e820 should presumably be aware of 
this, which it is not right now)


>
>> +static int init_acpi_config(struct xc_dom_image *dom,
>> +struct acpi_config *config)
>> +{
>> +xc_interface *xch = dom->xch;
>> +uint32_t domid = dom->guest_domid;
>> +xc_dominfo_t info;
>> +int i, rc;
>> +
>> +memset(config, 0, sizeof(*config));
>> +
>> +config->dsdt_anycpu = config->dsdt_15cpu = dsdt_empty;
>> +config->dsdt_anycpu_len = config->dsdt_15cpu_len = dsdt_empty_len;
> What good does an empty DSDT do? (Perhaps this question is a
> result of there not being any description of this change.)

DSDT is required to be present by the spec. And ACPICA gets upset if it
doesn't see it.

> +/* Copy acpi_info page into guest's memory */
> +extent = PFN(ACPI_INFO_PHYSICAL_ADDRESS);
> +rc = xc_domain_populate_physmap_exact(xch, domid, 1, 0, 0, );
> +if ( rc )
> +{
> +DOMPRINTF("%s: xc_domain_populate_physmap failed with %d\n",
> +  __FUNCTION__, rc);
> +goto out;
> +}
> +guest_info_page = xc_map_foreign_range(xch, domid, PAGE_SIZE,
> +   PROT_READ | PROT_WRITE,
> +   PFN(ACPI_INFO_PHYSICAL_ADDRESS));
> +if ( !guest_info_page )
> +{
> +DOMPRINTF("%s: Can't map acpi_info_page", __FUNCTION__);
> +rc = -1;
> +goto out;
> +}
> +memcpy(guest_info_page, acpi_pages, PAGE_SIZE);
> +
> +/* Copy ACPI tables into guest's memory */
> +acpi_pages_num = ((alloc_up - (unsigned long)acpi_pages +
> +   (PAGE_SIZE - 1)) >> PAGE_SHIFT) - 1;
> This looks like there are acpi_pages_num pages starting at
> acpi_pages, yet ...
>
>> +extents = malloc(acpi_pages_num * sizeof(*extents));
>> +if ( !extents )
>> +{
>> +DOMPRINTF("%s: Can't allocate extents array", __FUNCTION__);
>> +rc = -ENOMEM;
>> +goto out;
>> +}
>> +for (i = 0; i < acpi_pages_num; i++)
>> +extents[i] = PFN(RESERVED_MEMORY_DYNAMIC_START) + i;
>> +rc = xc_domain_populate_physmap_exact(xch, domid, acpi_pages_num,
>> +  0, 0, extents);
>> +if ( rc )
>> +{
>> +DOMPRINTF("%s: xc_domain_populate_physmap failed with %d",
>> +  __FUNCTION__, rc);
>> +goto out;
>> +}
>> +guest_acpi_pages = xc_map_foreign_range(xch, domid,
>> +PAGE_SIZE * acpi_pages_num,
>> +PROT_READ | PROT_WRITE,
>> +
>> PFN(RESERVED_MEMORY_DYNAMIC_START));
>> +if ( !guest_acpi_pages )
>> +{
>> +DOMPRINTF("%s Can't map guest_acpi_pages", __FUNCTION__);
>> +rc = -1;
>> +goto out;
>> +}
>> +
>> +memcpy(guest_acpi_pages, acpi_pages + PAGE_SIZE,
>> +   acpi_pages_num * PAGE_SIZE);
> ... this looks like there are acpi_pages_num pages starting at
> acpi_pages + PAGE_SIZE.

Yes, I am off-by-one here. The first page (ACPI_INFO_PHYSICAL_ADDRESS)
is managed separately.

>
>> --- a/tools/libxc/xc_dom_x86.c
>> +++ b/tools/libxc/xc_dom_x86.c
>> @@ -643,6 +643,13 @@ static int alloc_magic_pages_hvm(struct xc_dom_image 
>> *dom)
>>  DOMPRINTF("Unable to reserve memory for the start info");
>>  goto out;
>>  }
>> +
>> +rc = xc_dom_build_acpi(dom);
>> +if ( rc != 0 )
>> +{
>> +DOMPRINTF("Unable to build ACPI tables");
>> +goto out;
>> +}
> Iirc there is an "acpi=" guest config setting, yet neither here nor
> down the call tree 

[Xen-devel] Xen Security Advisory 178 (CVE-2016-4963) - Unsanitised driver domain input in libxl device handling

[Xen . org security team]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Xen Security Advisory CVE-2016-4963 / XSA-178
  version 4

   Unsanitised driver domain input in libxl device handling

UPDATES IN VERSION 4


Clarify that issue goes back as far as driver domain suppoprt.

Mention backports.

ISSUE DESCRIPTION
=

libxl's device-handling code freely uses and trusts information from
the backend directories in xenstore.

The backend domain (driver domain) can store bogus data in the
backend, causing libxl's enquiry functions to fail, confusing
management tools.

A driver domain can also remove its backend directory from xenstore
entirely, preventing the device from showing up in device listings and
preventing it from being removed and replaced.

A driver domain can cause libxl to generate disk eject events for
disks for which the driver domain is not responsible.

IMPACT
==

A malicious driver domain can deny service to management tools.

VULNERABLE SYSTEMS
==

This vulnerability is only applicable to systems which are using
driver domains, and then only where the driver domain is not intended
to be fully trusted with respect to the host.

Such Xen systems using libxl based toolstacks (for example xl or
libvirt with the libxl driver) are vulnerable.

Note that even with this vulnerability a driver domain based system is
better from a security point of view, than a system where devices are
provided directly by dom0.  Users and vendors of systems using driver
domains should not change their configuration.

All versions of libxl which support the relevant driver domains are
vulnerable.

MITIGATION
==

No mitigation is available.

CREDITS
===

This issue was discovered by Wei Liu from Citrix.

RESOLUTION
==

Applying the appropriate attached patch set from XSA-175, plus the
appropriate attached patch set below, resolves this issue.

xsa178-unstable/*.patch   xen-unstable

For earlier Xen releases, patches have been prepared and applied to
the staging-4.X branches, back to Xen 4.3.  The Xen Project CI system,
osstest, is currently doing basic functional testing of these
backports.  However, driver domains are not tested by osstest, nor
have they been tested by the Xen Project Security Team.  The patches
can be found in xen.git:
  xen.git commits 2805844bf20e..562ecb389444Xen 4.6
  xen.git commits 6265a6fc7f58..d8ac67eff778Xen 4.5
  xen.git commits 551948806424..a0b99aba04d9Xen 4.4
  xen.git commits 5811d6bdf5bb..08ea21f2361dXen 4.3
See:
  http://xenbits.xen.org/gitweb/?p=xen.git;a=summary
  git://xenbits.xen.org/xen.git
  http://xenbits.xen.org/git-http/xen.git

$ sha256sum xsa178-*/*
fd6a1f858d44f618a4e792553598005871f63d12e718bc9b5477d14bf0113386  
xsa178-unstable/0001-libxl-Make-copy-of-every-xs-backend-in-libxl-in-_gen.patch
ee6cf66ad385203c49d9b030959715fb885a250aa36b85080e6985a603bb1ddb  
xsa178-unstable/0002-libxl-Do-not-trust-backend-in-libxl__device_exists.patch
ea29cf28609c2d467fb7a620601af7bf434b098a7554dada956f11ed50c1b895  
xsa178-unstable/0003-libxl-Do-not-trust-backend-for-vtpm-in-getinfo-excep.patch
a2abc4308d9a18f49a02e6ca8ba913d4d9890867b7816dcc19b548836b65af6c  
xsa178-unstable/0004-libxl-Do-not-trust-backend-for-vtpm-in-getinfo-uuid.patch
2884e6566c59ae95792d4282e174c6b3d201c1e006b9e0ab57fbaad2b62ecfb9  
xsa178-unstable/0005-libxl-cdrom-eject-and-insert-write-to-libxl.patch
d6ac82211d056a386d18b8296a6a1f2e8a65e8156594595b9c34a3a377f1cf98  
xsa178-unstable/0006-libxl-Do-not-trust-backend-for-disk-eject-vdev.patch
4c8bb7bee3b624b02796afdfa0157ea1dc49a7f54f34912f992bae201b6bfe40  
xsa178-unstable/0007-libxl-Do-not-trust-backend-for-disk-fix-driver-domai.patch
556b14e8783ddd7ad0cb9a561ca43a40b37ccb27cd56337e7714ac0f796ce21b  
xsa178-unstable/0008-libxl-Do-not-trust-backend-for-disk-in-getinfo.patch
b51aaa8cca1f367ae51ffb65240831617d4cab4a3fa6d0a2d42728e99ee8cee8  
xsa178-unstable/0009-libxl-Do-not-trust-backend-for-cdrom-insert.patch
3ef493e6bda2d2b96a89cf18b55d43fbdb84a2cd5c10c88f04299434c629ba2b  
xsa178-unstable/0010-libxl-Do-not-trust-backend-for-channel-in-getinfo.patch
da4db890c9e73fca006bc381f2208f9bff0fc35990c4dd51d5db27072d33  
xsa178-unstable/0011-libxl-Rename-libxl__device_-nic-channel-_from_xs_be-.patch
ae8b043a83cc35beee2205ab621b6f5bc6543f6d4dcdc06c97e07b1a17ca94bf  
xsa178-unstable/0012-libxl-Rename-READ_BACKEND-to-READ_LIBXLDEV.patch
936c44de9a344b0634b7bff4f5b3cf9c034a0080e87d267e7a84683a967d1bff  
xsa178-unstable/0013-libxl-Have-READ_LIBXLDEV-use-libxl_path-rather-than-.patch
3b65a3140387651cf2ed1bcf8668efecd58fbd274a62a03d785c269b55bea8fe  
xsa178-unstable/0014-libxl-Do-not-trust-backend-in-nic-getinfo.patch
6d009153b98fd58f316efa4f39c821cf609b54184726e15f887947321610ed14  
xsa178-unstable/0015-libxl-Do-not-trust-backend-for-nic-in-devid_to_devic.patch
3105c062bb2017681f47499e2dd2f6cd2996539068f216a5af7d6143bc726eda  

Re: [Xen-devel] [PATCH RESEND 00/14] Xen ARM DomU ACPI support

[Boris Ostrovsky]

On 06/06/2016 11:37 AM, Boris Ostrovsky wrote:
> On 06/06/2016 08:26 AM, Wei Liu wrote:
>> On Tue, May 31, 2016 at 01:02:52PM +0800, Shannon Zhao wrote:
>>> From: Shannon Zhao 
>>>
>>> The design of this feature is described as below.
>>> Firstly, the toolstack (libxl) generates the ACPI tables according the
>>> number of vcpus and gic controller.
>>>
>>> Then, it copies these ACPI tables to DomU memory space and passes
>>> them to UEFI firmware through the "ARM multiboot" protocol.
>>>
>>> At last, UEFI gets the ACPI tables through the "ARM multiboot" protocol
>>> and installs these tables like the usual way and passes both ACPI and DT
>>> information to the Xen DomU.
>>>
>>> Currently libxl only generates RSDP, XSDT, GTDT, MADT, FADT, DSDT tables
>>> since it's enough now.
>>>
>>> This has been tested using guest kernel with the Dom0 ACPI support
>>> patches which could be fetched from:
>>> https://git.kernel.org/cgit/linux/kernel/git/mfleming/efi.git/log/?h=efi/arm-xen
>>>
>>> Shannon Zhao (14):
>>>   libxl/arm: Fix the function name in error log
>>>   libxl/arm: Factor out codes for generating DTB
>>>   libxc: Add placeholders for ACPI tables blob and size
>>>   tools: add ACPI tables relevant definitions
>>>   libxl/arm: Construct ACPI GTDT table
>>>   libxl/arm: Construct ACPI FADT table
>>>   libxl/arm: Construct ACPI DSDT table
>>>   libxl/arm: Construct ACPI MADT table
>>>   libxl/arm: Construct ACPI XSDT table
>>>   libxl/arm: Construct ACPI RSDP table
>>>   libxl/arm: Initialize domain param HVM_PARAM_CALLBACK_IRQ
>>>   libxl/arm: Add ACPI module
>>>   libxl/arm: initialize memory information of ACPI blob
>>>   libxc/xc_dom_core: Copy ACPI tables to guest memory space
>>>
>> After going through this series, I think the code mostly looks good.
>>
>> There is one higher level question: you seem to have put a lot of the
>> table construction code in libxl, while I failed to see why specific
>> libxl information is needed. Have you considered moving the code to
>> libxc?
>>
>> What I don't want to see is that x86 builds its ACPI table in libxc
>> while ARM builds its in libxl. That would make future merger harder.
>>
>> Boris?
> TBH, I am not sure which library this should really belong to. I felt
> libxc would be more appropriate.

Actually, I now remember why I picked libxc: because I wanted ACPI pages
to be loaded as part of xc_dom_build_image().

-boris



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

[Xen-devel] [qemu-upstream-4.3-testing test] 95327: trouble: blocked/broken

[osstest service owner]

flight 95327 qemu-upstream-4.3-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/95327/

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64   3 host-install(3) broken REGR. vs. 80927
 build-i386-pvops  3 host-install(3) broken REGR. vs. 80927
 build-i3863 host-install(3) broken REGR. vs. 80927
 build-amd64-pvops 3 host-install(3) broken REGR. vs. 80927

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  1 build-check(1)blocked n/a
 build-amd64-libvirt   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-xl-qemuu-winxpsp3-vcpus1  1 build-check(1) blocked n/a
 test-amd64-i386-xl-raw1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-win7-amd64  1 build-check(1)  blocked n/a
 test-amd64-amd64-xl-credit2   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-i386-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-i386  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-amd64  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-win7-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-pygrub   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-winxpsp3  1 build-check(1)   blocked n/a
 test-amd64-amd64-xl-qcow2 1 build-check(1)   blocked  n/a
 test-amd64-amd64-amd64-pvgrub  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl   1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-debianhvm-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-qemuu-rhel6hvm-intel  1 build-check(1) blocked n/a
 test-amd64-i386-xl1 build-check(1)   blocked  n/a
 test-amd64-i386-qemuu-rhel6hvm-amd  1 build-check(1)   blocked n/a
 test-amd64-i386-pv1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-vhd  1 build-check(1)   blocked  n/a
 test-amd64-amd64-pair 1 build-check(1)   blocked  n/a
 test-amd64-amd64-pv   1 build-check(1)   blocked  n/a

version targeted for testing:
 qemuuc97c20f71240a538a19cb6b0e598bc1bbd5168f1
baseline version:
 qemuu10c1b763c26feb645627a1639e722515f3e1e876

Last test of basis80927  2016-02-06 13:30:02 Z  121 days
Testing same since93977  2016-05-10 11:09:16 Z   27 days  114 attempts


People who touched revisions under test:
  Gerd Hoffmann 
  Stefano Stabellini 

jobs:
 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  blocked 
 test-amd64-i386-xl   blocked 
 test-amd64-i386-qemuu-rhel6hvm-amd   blocked 
 test-amd64-amd64-xl-qemuu-debianhvm-amd64blocked 
 test-amd64-i386-xl-qemuu-debianhvm-amd64 blocked 
 test-amd64-i386-freebsd10-amd64  blocked 
 test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked 
 test-amd64-i386-xl-qemuu-ovmf-amd64  blocked 
 test-amd64-amd64-xl-qemuu-win7-amd64 blocked 
 test-amd64-i386-xl-qemuu-win7-amd64  blocked 
 test-amd64-amd64-xl-credit2  blocked 
 test-amd64-i386-freebsd10-i386   blocked 
 test-amd64-i386-qemuu-rhel6hvm-intel blocked 
 test-amd64-amd64-libvirt blocked 
 test-amd64-i386-libvirt  blocked 
 test-amd64-amd64-xl-multivcpublocked 
 

[Xen-devel] Xen, systemd, and selinux

[George Dunlap]

Hey Michael,

Not sure if you know, I've been maintaining the Xen4CentOS packages; I
suspect given the similarities between our systems we're solving the
same issues; particularly with the systemd/selinux combination.

I've just ported my patchqueue up to 4.7-rc4, and it looks like the
SELinux rules for xenstored -- at least the ones that come with CentOS
7 -- are outdated; they allow xenstored to open /proc/xen/privcmd
(which is deprecated), but not /dev/xen/privcmd.

Do you know where the "upstream" for these rules are, and how to get
them changed in a way that will trickle down eventually to CentOS?

As of 4.7-rc4, libxc will first try to open /dev/xen/privcmd, then
*if* it fails with a certain set of error codes, it tries
/proc/xen/privcmd instead.  Unfortunately, EACCES (the failure you get
from SELinux denials) is not one of those error codes.  If you just
add that error code in to the list of acceptable error codes, then
things work for me.

Thanks,
 -George

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

Re: [Xen-devel] [PATCH v5 5/9] monitor: ARM SMC events

[Julien Grall]

Hi Tamas,

On 06/06/16 17:14, Tamas K Lengyel wrote:

On Mon, Jun 6, 2016 at 9:56 AM, Tamas K Lengyel  wrote:

On Mon, Jun 6, 2016 at 9:54 AM, Julien Grall  wrote:

So either way, I don't see a technical reason why Xen should silently
swallow any SMC trap if the vm_event user specifically asked them to
be forwarded. Other then it being odd that some ARM chips have varying
behavior regarding a subset of SMC instructions, it should not affect
when the vm_event user gets the events. If the user requests that it
wants to get notified any time an SMC is trapped to the VMM, it
should, regardless of whether that makes sense for "us". Depending on
the use-case of the user, indeed it may need extra information if it
wants to do emulation. If that need arises, the interface can easily
be extended to accommodate that usecase. We can also add a comment
saying that the forwarded events may also include ones with failed
condition checks depending on the CPU implementation. Also, it would
also be possible in the future to add a monitor configuration bit
where the user can specify if it wants the failed condition check SMCs
ignored by default or not. At this time however I want to start simple
and just forward all events, adding more bits and pieces only as
needed.


We disagree on what is a "starting simple". It easier to relax than 
restricting a behavior later one.


Even if we decide to add a bit to ignore some SMC in a later version of 
Xen, the introspection app will need to carry the burden mentioned in 
lengthly way on the previous mails because they may want to support 
older version of Xen.


It would not be that difficult to provide a clean interface from 
beginning, which would allow to support more than your usecase.


Anyway, I am not gonna ack this patch for the modification in 
arch/arm/traps.c because I don't think this is the right way to go. I 
will let Stefano deciding on this one.


Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v5 5/9] monitor: ARM SMC events

[Tamas K Lengyel]

On Mon, Jun 6, 2016 at 9:56 AM, Tamas K Lengyel  wrote:
> On Mon, Jun 6, 2016 at 9:54 AM, Julien Grall  wrote:
>> Hello Tamas,
>>
>> On 06/06/16 16:24, Tamas K Lengyel wrote:
>>>
>>>
>>> On Jun 6, 2016 04:08, "Julien Grall" >> > wrote:
>>>  >
>>>  > Hello,
>>>  >
>>>  >
>>>  > On 04/06/2016 18:40, Tamas K Lengyel wrote:
>>>  >>
>>>  >> On Sat, Jun 4, 2016 at 3:03 AM, Edgar E. Iglesias
>>>  >> > wrote:
>>>  >>>
>>>  >>> Forwarding SMC events for SMC insns that didn't pass the condition
>>> tests
>>>  >>> doesn't make any sense to me. It'll just make the receivers job
>>> harder.
>>>  >>> Why would a receiver want to do anything else than drop these?
>>>  >>> If it actually does look at them it'll be looking at implementation
>>>  >>> defined HW behaviour that may vary between CPU implementations.
>>>  >>
>>>  >>
>>>  >> If for no other purposes it may be useful to log them to be able to
>>>  >> study the CPU implementation's behavior.
>>>  >
>>>  >
>>>  > I cannot see how you will be able to study ARM CPU implementation's
>>> behavior with VM event. Though I am not familiar with it.
>>>  >
>>>  > For now, it looks like to me that forwarding conditional SMC even if
>>> the condition check has failed will require a lot of code in each
>>> introspection applications, not to mention that they will need specific
>>> code to distinguish ARMv7 vs ARMv8.
>>>
>>> Why would it require any more code? Right now the only thing the
>>> listener can do is to increment pc to jump over the SMC. That would be
>>> the same regardless of what type it was. As for checking whether it was
>>> v7 or v8, if that is of interest to the app it should implement the
>>> appropriate logic for it. I don't see a problem there either.
>>
>>
>> A listener can to do more then incrementing pc to jump over the SMC. The app
>> may want to decode the instruction to get the immediate and then execute
>> different actions depending on its value (for instance to emulate some SMC
>> call).
>>
>> For this kind of app, it will be necessary to find out whether the condition
>> check has failed or not before executing it.
>>
>
> Sure, and if that need arises the extra information can also be
> forwarded. Especially if as you said you are planning to implement the
> decoding for Xen to see if it was a failed condition check or not.
>

So either way, I don't see a technical reason why Xen should silently
swallow any SMC trap if the vm_event user specifically asked them to
be forwarded. Other then it being odd that some ARM chips have varying
behavior regarding a subset of SMC instructions, it should not affect
when the vm_event user gets the events. If the user requests that it
wants to get notified any time an SMC is trapped to the VMM, it
should, regardless of whether that makes sense for "us". Depending on
the use-case of the user, indeed it may need extra information if it
wants to do emulation. If that need arises, the interface can easily
be extended to accommodate that usecase. We can also add a comment
saying that the forwarded events may also include ones with failed
condition checks depending on the CPU implementation. Also, it would
also be possible in the future to add a monitor configuration bit
where the user can specify if it wants the failed condition check SMCs
ignored by default or not. At this time however I want to start simple
and just forward all events, adding more bits and pieces only as
needed.

Tamas

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

Re: [Xen-devel] [PATCH RFC 15/20] acpi: Move ACPI code to xen/common/libacpi

[Boris Ostrovsky]

On 06/06/2016 09:05 AM, Jan Beulich wrote:
 On 06.04.16 at 03:25,  wrote:
>>  .gitignore| 8 
>> 
>>  tools/firmware/hvmloader/Makefile | 3 +--
>>  tools/firmware/hvmloader/smbios.c | 1 +
>>  tools/firmware/rombios/32bit/Makefile | 2 +-
>>  tools/firmware/rombios/32bit/tcgbios/Makefile | 2 +-
>>  {tools/firmware/hvmloader/acpi => xen/common/libacpi}/Makefile| 6 +++---
>>  {tools/firmware/hvmloader/acpi => xen/common/libacpi}/README  | 0
>>  {tools/firmware/hvmloader/acpi => xen/common/libacpi}/acpi2_0.h   | 0
>>  {tools/firmware/hvmloader/acpi => xen/common/libacpi}/build.c | 0
>>  {tools/firmware/hvmloader/acpi => xen/common/libacpi}/dsdt.asl| 0
>>  {tools/firmware/hvmloader/acpi => xen/common/libacpi}/mk_dsdt.c   | 0
>>  {tools/firmware/hvmloader/acpi => xen/common/libacpi}/ssdt_pm.asl | 0
>>  {tools/firmware/hvmloader/acpi => xen/common/libacpi}/ssdt_s3.asl | 0
>>  {tools/firmware/hvmloader/acpi => xen/common/libacpi}/ssdt_s4.asl | 0
>>  .../firmware/hvmloader/acpi => xen/common/libacpi}/ssdt_tpm.asl   | 0
>>  .../hvmloader/acpi => xen/common/libacpi}/static_tables.c | 0
>>  {tools/firmware/hvmloader/acpi => xen/common/libacpi}/x86.h   | 0
>>  17 files changed, 11 insertions(+), 11 deletions(-)
> As mentioned before, new placement subject to determination
> whether this is to eventually be used by the hypervisor.


Roger, when do you think you'll be able to see whether dom0 builder will
use this? I can start working on v1 with assumption that this will be
used by hypervisor. We can drop the last patch (and modify this one to
move acpi to somewhere in tools) if it becomes clear that hypervisor
does not want it.


>
>> --- a/tools/firmware/hvmloader/acpi/Makefile
>> +++ b/xen/common/libacpi/Makefile
>> @@ -14,13 +14,13 @@
>>  # this program; If not, see .
>>  #
>>  
>> -XEN_ROOT = $(CURDIR)/../../../..
>> -include $(XEN_ROOT)/tools/firmware/Rules.mk
>> +XEN_ROOT = $(CURDIR)/../../..
>> +include $(XEN_ROOT)/Config.mk
>>  
>>  C_SRC = build.c dsdt_anycpu.c dsdt_15cpu.c static_tables.c 
>> dsdt_anycpu_qemu_xen.c
>>  OBJS  = $(patsubst %.c,%.o,$(C_SRC))
>>  
>> -CFLAGS += $(CFLAGS_xeninclude)
>> +CFLAGS_xeninclude = -I$(XEN_ROOT)/tools/include
> Where would this now get consumed? This appears to be a tools/
> only setting, which should thus no longer be needed here.

This is needed for building mk_dsdt.

-boris



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

Re: [Xen-devel] [PATCH v3] arm/acpi: Add Server Base System Architecture UART support

[Julien Grall]

Hi Shanker,

On 03/06/16 18:39, Shanker Donthineni wrote:

The ARM Server Base System Architecture describes a generic UART
interface. It doesn't support clock control registers, modem
control, DMA and hardware flow control features. So, extend the
driver probe() to handle SBSA interface and skip the accessing
PL011 registers that are not described in SBSA document
(ARM-DEN-0029 Version 3.0, 6 APPENDIX B: GENERIC UART).

Signed-off-by: Shanker Donthineni 
---
Changes since v2:
Edited commit text to include SBSA document version.
Remove setting baudrate code completely as per Julien's suggestion.


This item and ...


Support both the SBSA interface types ACPI_DBG2_SBSA & ACPI_DBG2_SBSA_32.
Replace MIS references with combination of RIS & IMSC.


this one need some description in the commit message to explain why it 
is fine for non-SBSA UART to do it.


The later also needs some explanation why MIS could be replaced by RIS & 
IMSC.


Lastly, IHMO those 2 items should be in separate patch to make the 
review easier.




Changes since v1:
Don't access UART registers that are not part of SBSA document.
Move setting baudrate function to a separate function.

  xen/drivers/char/pl011.c | 76 ++--
  1 file changed, 41 insertions(+), 35 deletions(-)

diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
index 1212d5c..3497d61 100644
--- a/xen/drivers/char/pl011.c
+++ b/xen/drivers/char/pl011.c
@@ -31,7 +31,7 @@
  #include 

  static struct pl011 {
-unsigned int baud, clock_hz, data_bits, parity, stop_bits;
+unsigned int data_bits, parity, stop_bits;
  unsigned int irq;
  void __iomem *regs;
  /* UART with IRQ line: interrupt-driven I/O. */
@@ -41,6 +41,7 @@ static struct pl011 {
  /* struct timer timer; */
  /* unsigned int timeout_ms; */
  /* bool_t probing, intr_works; */
+bool sbsa;  /* ARM SBSA generic interface */
  } pl011_com = {0};

  /* These parity settings can be ORed directly into the LCR. */
@@ -57,7 +58,9 @@ static void pl011_interrupt(int irq, void *data, struct 
cpu_user_regs *regs)
  {
  struct serial_port *port = data;
  struct pl011 *uart = port->uart;
-unsigned int status = pl011_read(uart, MIS);
+unsigned int status = pl011_read(uart, RIS);
+
+status &= pl011_read(uart, IMSC);

  if ( status )
  {
@@ -76,7 +79,7 @@ static void pl011_interrupt(int irq, void *data, struct 
cpu_user_regs *regs)
  if ( status & (TXI) )
  serial_tx_interrupt(port, regs);

-status = pl011_read(uart, MIS);
+status = pl011_read(uart, RIS) & pl011_read(uart, IMSC);


Please introduce a helper to read the status.


  } while (status != 0);
  }
  }


Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v5 5/9] monitor: ARM SMC events

[Julien Grall]

Hello Tamas,

On 06/06/16 16:24, Tamas K Lengyel wrote:


On Jun 6, 2016 04:08, "Julien Grall" > wrote:
 >
 > Hello,
 >
 >
 > On 04/06/2016 18:40, Tamas K Lengyel wrote:
 >>
 >> On Sat, Jun 4, 2016 at 3:03 AM, Edgar E. Iglesias
 >> > wrote:
 >>>
 >>> Forwarding SMC events for SMC insns that didn't pass the condition
tests
 >>> doesn't make any sense to me. It'll just make the receivers job harder.
 >>> Why would a receiver want to do anything else than drop these?
 >>> If it actually does look at them it'll be looking at implementation
 >>> defined HW behaviour that may vary between CPU implementations.
 >>
 >>
 >> If for no other purposes it may be useful to log them to be able to
 >> study the CPU implementation's behavior.
 >
 >
 > I cannot see how you will be able to study ARM CPU implementation's
behavior with VM event. Though I am not familiar with it.
 >
 > For now, it looks like to me that forwarding conditional SMC even if
the condition check has failed will require a lot of code in each
introspection applications, not to mention that they will need specific
code to distinguish ARMv7 vs ARMv8.

Why would it require any more code? Right now the only thing the
listener can do is to increment pc to jump over the SMC. That would be
the same regardless of what type it was. As for checking whether it was
v7 or v8, if that is of interest to the app it should implement the
appropriate logic for it. I don't see a problem there either.


A listener can to do more then incrementing pc to jump over the SMC. The 
app may want to decode the instruction to get the immediate and then 
execute different actions depending on its value (for instance to 
emulate some SMC call).


For this kind of app, it will be necessary to find out whether the 
condition check has failed or not before executing it.


Regards,

--
Julien Grall

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

[Xen-devel] [PATCH 2/6] xenconsoled: switch hypervisor log to use logfile abstraction

[Wei Liu]

To minimise code churn, copy and paste a some existing functions and
adapt them to write to logfile. The functions to deal with fd directly
will go away eventually.

Signed-off-by: Wei Liu 
---
 tools/console/daemon/io.c | 95 +++
 1 file changed, 71 insertions(+), 24 deletions(-)

diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
index 7e6a886..228c4af 100644
--- a/tools/console/daemon/io.c
+++ b/tools/console/daemon/io.c
@@ -21,6 +21,7 @@
 
 #include "utils.h"
 #include "io.h"
+#include "logfile.h"
 #include 
 #include 
 #include 
@@ -71,7 +72,7 @@ extern int discard_overflowed_data;
 
 static int log_time_hv_needts = 1;
 static int log_time_guest_needts = 1;
-static int log_hv_fd = -1;
+static struct logfile *log_hv_file;
 
 static xengnttab_handle *xgt_handle = NULL;
 
@@ -127,6 +128,15 @@ static int write_all(int fd, const char* buf, size_t len)
return 0;
 }
 
+static int write_logfile(struct logfile *logfile, const char *buf,
+size_t len)
+{
+   ssize_t r = logfile_append(logfile, buf, len);
+
+   if (r < 0) return -1;
+   return 0;
+}
+
 static int write_with_timestamp(int fd, const char *data, size_t sz,
int *needts)
 {
@@ -158,6 +168,38 @@ static int write_with_timestamp(int fd, const char *data, 
size_t sz,
return 0;
 }
 
+static int write_logfile_with_timestamp(struct logfile *logfile,
+   const char *data, size_t sz,
+   int *needts)
+{
+   char ts[32];
+   time_t now = time(NULL);
+   const struct tm *tmnow = localtime();
+   size_t tslen = strftime(ts, sizeof(ts), "[%Y-%m-%d %H:%M:%S] ", tmnow);
+   const char *last_byte = data + sz - 1;
+
+   while (data <= last_byte) {
+   const char *nl = memchr(data, '\n', last_byte + 1 - data);
+   int found_nl = (nl != NULL);
+   if (!found_nl)
+   nl = last_byte;
+
+   if ((*needts && logfile_append(logfile, ts, tslen))
+   || logfile_append(logfile, data, nl + 1 - data))
+   return -1;
+
+   *needts = found_nl;
+   data = nl + 1;
+   if (found_nl) {
+   // If we printed a newline, strip all \r following it
+   while (data <= last_byte && *data == '\r')
+   data++;
+   }
+   }
+
+   return 0;
+}
+
 static void buffer_append(struct domain *dom)
 {
struct buffer *buffer = >buffer;
@@ -265,29 +307,33 @@ static bool domain_is_valid(int domid)
return ret;
 }
 
-static int create_hv_log(void)
+static struct logfile *create_hv_log(void)
 {
char logfile[PATH_MAX];
-   int fd;
+   struct logfile *tmp;
+
snprintf(logfile, PATH_MAX-1, "%s/hypervisor.log", log_dir);
logfile[PATH_MAX-1] = '\0';
 
-   fd = open(logfile, O_WRONLY|O_CREAT|O_APPEND, 0644);
-   if (fd == -1)
+   tmp = logfile_new(logfile, 0644);
+
+   if (!tmp)
dolog(LOG_ERR, "Failed to open log %s: %d (%s)",
  logfile, errno, strerror(errno));
-   if (fd != -1 && log_time_hv) {
-   if (write_with_timestamp(fd, "Logfile Opened\n",
-strlen("Logfile Opened\n"),
-_time_hv_needts) < 0) {
+
+   if (tmp && log_time_hv) {
+   if (write_logfile_with_timestamp(tmp, "Logfile Opened\n",
+strlen("Logfile Opened\n"),
+_time_hv_needts) < 0) {
dolog(LOG_ERR, "Failed to log opening timestamp "
-  "in %s: %d (%s)", logfile, errno,
+  "in %s: %d (%s)", tmp->basepath, errno,
   strerror(errno));
-   close(fd);
-   return -1;
+   logfile_free(tmp);
+   return NULL;
}
}
-   return fd;
+
+   return tmp;
 }
 
 static int create_domain_log(struct domain *dom)
@@ -929,10 +975,11 @@ static void handle_hv_logs(xenevtchn_handle *xce_handle, 
bool force)
break;
 
if (log_time_hv)
-   logret = write_with_timestamp(log_hv_fd, buffer, size,
- _time_hv_needts);
+   logret = write_logfile_with_timestamp(log_hv_file,
+ buffer, size,
+ 
_time_hv_needts);
else
-   logret = write_all(log_hv_fd, buffer, size);
+   

[Xen-devel] [PATCH 4/6] xenconsoled: delete two now unused functions

[Wei Liu]

Signed-off-by: Wei Liu 
---
 tools/console/daemon/io.c | 47 ---
 1 file changed, 47 deletions(-)

diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
index c2f63e6..7c38232 100644
--- a/tools/console/daemon/io.c
+++ b/tools/console/daemon/io.c
@@ -113,21 +113,6 @@ struct domain {
 
 static struct domain *dom_head;
 
-static int write_all(int fd, const char* buf, size_t len)
-{
-   while (len) {
-   ssize_t ret = write(fd, buf, len);
-   if (ret == -1 && errno == EINTR)
-   continue;
-   if (ret <= 0)
-   return -1;
-   len -= ret;
-   buf += ret;
-   }
-
-   return 0;
-}
-
 static int write_logfile(struct logfile *logfile, const char *buf,
 size_t len)
 {
@@ -137,38 +122,6 @@ static int write_logfile(struct logfile *logfile, const 
char *buf,
return 0;
 }
 
-static  __attribute__((unused))
-int write_with_timestamp(int fd, const char *data, size_t sz,
-int *needts)
-{
-   char ts[32];
-   time_t now = time(NULL);
-   const struct tm *tmnow = localtime();
-   size_t tslen = strftime(ts, sizeof(ts), "[%Y-%m-%d %H:%M:%S] ", tmnow);
-   const char *last_byte = data + sz - 1;
-
-   while (data <= last_byte) {
-   const char *nl = memchr(data, '\n', last_byte + 1 - data);
-   int found_nl = (nl != NULL);
-   if (!found_nl)
-   nl = last_byte;
-
-   if ((*needts && write_all(fd, ts, tslen))
-   || write_all(fd, data, nl + 1 - data))
-   return -1;
-
-   *needts = found_nl;
-   data = nl + 1;
-   if (found_nl) {
-   // If we printed a newline, strip all \r following it
-   while (data <= last_byte && *data == '\r')
-   data++;
-   }
-   }
-
-   return 0;
-}
-
 static int write_logfile_with_timestamp(struct logfile *logfile,
const char *data, size_t sz,
int *needts)
-- 
2.1.4


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

[Xen-devel] [PATCH 5/6] xenconsoled: options to control log rotation

[Wei Liu]

Provide two options: one to control the maximum number of copies kept
and the other to control the maximum length of each log file.

Signed-off-by: Wei Liu 
---
 tools/console/daemon/logfile.c | 11 +++
 tools/console/daemon/main.c| 13 -
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/tools/console/daemon/logfile.c b/tools/console/daemon/logfile.c
index ad73f8e..3b95f84 100644
--- a/tools/console/daemon/logfile.c
+++ b/tools/console/daemon/logfile.c
@@ -25,6 +25,9 @@
 #include "utils.h"
 #include "logfile.h"
 
+extern unsigned int log_backups;
+extern unsigned int log_max_len;
+
 static void logfile_entry_free(struct logfile_entry *entry)
 {
if (!entry) return;
@@ -91,7 +94,7 @@ struct logfile *logfile_new(const char *path, mode_t mode)
}
 
logfile->mode = mode;
-   logfile->maxlen = LOGFILE_MAX_SIZE;
+   logfile->maxlen = log_max_len;
 
 
logfile->entry = logfile_entry_new(logfile->basepath, mode);
@@ -112,13 +115,13 @@ static int logfile_rollover(struct logfile *file)
unsigned i;
 
if (asprintf(, "%s.%u", file->basepath,
-LOGFILE_MAX_BACKUP_NUM) < 0) {
+log_backups) < 0) {
dolog(LOG_ERR, "Failed to asprintf %s.%u",
- file->basepath, LOGFILE_MAX_BACKUP_NUM);
+ file->basepath, log_backups);
goto err;
}
 
-   for (i = LOGFILE_MAX_BACKUP_NUM; i > 0; i--) {
+   for (i = log_backups; i > 0; i--) {
if (i == 1) {
this = strdup(file->basepath);
if (!this) {
diff --git a/tools/console/daemon/main.c b/tools/console/daemon/main.c
index 23860d3..dd338ae 100644
--- a/tools/console/daemon/main.c
+++ b/tools/console/daemon/main.c
@@ -31,6 +31,7 @@
 
 #include "utils.h"
 #include "io.h"
+#include "logfile.h"
 
 int log_reload = 0;
 int log_guest = 0;
@@ -39,6 +40,8 @@ int log_time_hv = 0;
 int log_time_guest = 0;
 char *log_dir = NULL;
 int discard_overflowed_data = 1;
+unsigned int log_backups = LOGFILE_MAX_BACKUP_NUM;
+unsigned int log_max_len = LOGFILE_MAX_SIZE;
 
 static void handle_hup(int sig)
 {
@@ -47,7 +50,7 @@ static void handle_hup(int sig)
 
 static void usage(char *name)
 {
-   printf("Usage: %s [-h] [-V] [-v] [-i] [--log=none|guest|hv|all] 
[--log-dir=DIR] [--pid-file=PATH] [-t, --timestamp=none|guest|hv|all] [-o, 
--overflow-data=discard|keep]\n", name);
+   printf("Usage: %s [-h] [-V] [-v] [-i] [--log=none|guest|hv|all] 
[--log-dir=DIR] [--pid-file=PATH] [--log-backups=NUM] [--log-max-len=NUM] [-t, 
--timestamp=none|guest|hv|all] [-o, --overflow-data=discard|keep]\n", name);
 }
 
 static void version(char *name)
@@ -100,6 +103,8 @@ int main(int argc, char **argv)
{ "interactive", 0, 0, 'i' },
{ "log", 1, 0, 'l' },
{ "log-dir", 1, 0, 'r' },
+   { "log-backups", 1, 0, 'n' },
+   { "log-max-len", 1, 0, 'm' },
{ "pid-file", 1, 0, 'p' },
{ "timestamp", 1, 0, 't' },
{ "overflow-data", 1, 0, 'o'},
@@ -144,6 +149,12 @@ int main(int argc, char **argv)
case 'r':
log_dir = strdup(optarg);
break;
+   case 'n':
+   log_backups = strtoul(optarg, 0, 0);
+   break;
+   case 'm':
+   log_max_len = strtoul(optarg, 0, 0);
+   break;
case 'p':
pidfile = strdup(optarg);
break;
-- 
2.1.4


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

[Xen-devel] [PATCH 0/6] xenconsoled: rotating log file abstration

[Wei Liu]

Wei Liu (6):
  xenconsoled: introduce log file abstraction
  xenconsoled: switch hypervisor log to use logfile abstraction
  xenconsoled: switch guest log to use logfile abstraction
  xenconsoled: delete two now unused functions
  xenconsoled: options to control log rotation
  xenconsoled: handle --log-backups 0 in logfile_rollover

 tools/console/daemon/io.c  | 139 +++
 tools/console/daemon/logfile.c | 243 +
 tools/console/daemon/logfile.h |  57 ++
 tools/console/daemon/main.c|  13 ++-
 4 files changed, 383 insertions(+), 69 deletions(-)
 create mode 100644 tools/console/daemon/logfile.c
 create mode 100644 tools/console/daemon/logfile.h

-- 
2.1.4


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

[Xen-devel] [PATCH 6/6] xenconsoled: handle --log-backups 0 in logfile_rollover

[Wei Liu]

We now allow user to configure the number of backups to keep. We need to
handle when the number is set to 0.

Check if number of backup is 0. If so, just unlink the file. Move the
rotation to `else' branch so that we skip it altogether.

Signed-off-by: Wei Liu 
---
 tools/console/daemon/logfile.c | 63 +-
 1 file changed, 37 insertions(+), 26 deletions(-)

diff --git a/tools/console/daemon/logfile.c b/tools/console/daemon/logfile.c
index 3b95f84..fd29ba5 100644
--- a/tools/console/daemon/logfile.c
+++ b/tools/console/daemon/logfile.c
@@ -114,38 +114,49 @@ static int logfile_rollover(struct logfile *file)
char *next = NULL;
unsigned i;
 
-   if (asprintf(, "%s.%u", file->basepath,
-log_backups) < 0) {
-   dolog(LOG_ERR, "Failed to asprintf %s.%u",
- file->basepath, log_backups);
-   goto err;
-   }
+   if (log_backups == 0) {
+   if (unlink(file->basepath) < 0 &&
+   errno != ENOENT) {
+   dolog(LOG_ERR, "Failed to unlink %s",
+ file->basepath);
+   goto err;
+   }
+   } else {
+   if (asprintf(, "%s.%u", file->basepath,
+log_backups) < 0) {
+   dolog(LOG_ERR, "Failed to asprintf %s.%u",
+ file->basepath, log_backups);
+   goto err;
+   }
 
-   for (i = log_backups; i > 0; i--) {
-   if (i == 1) {
-   this = strdup(file->basepath);
-   if (!this) {
-   dolog(LOG_ERR, "Failed to strdup %s",
- file->basepath);
-   goto err;
+   for (i = log_backups; i > 0; i--) {
+   if (i == 1) {
+   this = strdup(file->basepath);
+   if (!this) {
+   dolog(LOG_ERR, "Failed to strdup %s",
+ file->basepath);
+   goto err;
+   }
+   } else {
+   if (asprintf(, "%s.%u", file->basepath,
+i-1) < 0) {
+   dolog(LOG_ERR,
+ "Failed to asprintf %s.%u",
+ file->basepath, i-1);
+   goto err;
+   }
}
-   } else {
-   if (asprintf(, "%s.%u", file->basepath, i-1) < 0) {
-   dolog(LOG_ERR, "Failed to asprintf %s.%u",
- file->basepath, i-1);
+
+   if (rename(this, next) < 0 && errno != ENOENT) {
+   dolog(LOG_ERR, "Failed to rename %s to %s",
+ this, next);
goto err;
}
-   }
 
-   if (rename(this, next) < 0 && errno != ENOENT) {
-   dolog(LOG_ERR, "Failed to rename %s to %s",
- this, next);
-   goto err;
+   free(next);
+   next = this;
+   this = NULL;
}
-
-   free(next);
-   next = this;
-   this = NULL;
}
 
new = logfile_entry_new(file->basepath, file->mode);
-- 
2.1.4


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

[Xen-devel] [PATCH 3/6] xenconsoled: switch guest log to use logfile abstraction

[Wei Liu]

Note that this causes write_with_timestamp to have no caller. Mark
it as unused for now to minimise code churn.

Signed-off-by: Wei Liu 
---
 tools/console/daemon/io.c | 67 +--
 1 file changed, 35 insertions(+), 32 deletions(-)

diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
index 228c4af..c2f63e6 100644
--- a/tools/console/daemon/io.c
+++ b/tools/console/daemon/io.c
@@ -95,7 +95,7 @@ struct domain {
int master_fd;
int master_pollfd_idx;
int slave_fd;
-   int log_fd;
+   struct logfile *log;
bool is_dead;
unsigned last_seen;
struct buffer buffer;
@@ -137,8 +137,9 @@ static int write_logfile(struct logfile *logfile, const 
char *buf,
return 0;
 }
 
-static int write_with_timestamp(int fd, const char *data, size_t sz,
-   int *needts)
+static  __attribute__((unused))
+int write_with_timestamp(int fd, const char *data, size_t sz,
+int *needts)
 {
char ts[32];
time_t now = time(NULL);
@@ -235,16 +236,16 @@ static void buffer_append(struct domain *dom)
 * no one is listening on the console pty then it will fill up
 * and handle_tty_write will stop being called.
 */
-   if (dom->log_fd != -1) {
+   if (dom->log) {
int logret;
if (log_time_guest) {
-   logret = write_with_timestamp(
-   dom->log_fd,
+   logret = write_logfile_with_timestamp(
+   dom->log,
buffer->data + buffer->size - size,
size, _time_guest_needts);
} else {
-   logret = write_all(
-   dom->log_fd,
+   logret = write_logfile(
+   dom->log,
buffer->data + buffer->size - size,
size);
}
@@ -336,50 +337,53 @@ static struct logfile *create_hv_log(void)
return tmp;
 }
 
-static int create_domain_log(struct domain *dom)
+static struct logfile *create_domain_log(struct domain *dom)
 {
char logfile[PATH_MAX];
char *namepath, *data, *s;
-   int fd;
unsigned int len;
+   struct logfile *tmp;
 
namepath = xs_get_domain_path(xs, dom->domid);
s = realloc(namepath, strlen(namepath) + 6);
if (s == NULL) {
free(namepath);
-   return -1;
+   return NULL;
}
namepath = s;
strcat(namepath, "/name");
data = xs_read(xs, XBT_NULL, namepath, );
free(namepath);
if (!data)
-   return -1;
+   return NULL;
if (!len) {
free(data);
-   return -1;
+   return NULL;
}
 
snprintf(logfile, PATH_MAX-1, "%s/guest-%s.log", log_dir, data);
free(data);
logfile[PATH_MAX-1] = '\0';
 
-   fd = open(logfile, O_WRONLY|O_CREAT|O_APPEND, 0644);
-   if (fd == -1)
+   tmp = logfile_new(logfile, 0644);
+
+   if (!tmp)
dolog(LOG_ERR, "Failed to open log %s: %d (%s)",
  logfile, errno, strerror(errno));
-   if (fd != -1 && log_time_guest) {
-   if (write_with_timestamp(fd, "Logfile Opened\n",
-strlen("Logfile Opened\n"),
-_time_guest_needts) < 0) {
+
+   if (tmp && log_time_guest) {
+   if (write_logfile_with_timestamp(tmp, "Logfile Opened\n",
+strlen("Logfile Opened\n"),
+_time_guest_needts) < 0) {
dolog(LOG_ERR, "Failed to log opening timestamp "
-  "in %s: %d (%s)", logfile, errno,
+  "in %s: %d (%s)", tmp->basepath, errno,
   strerror(errno));
-   close(fd);
-   return -1;
+   logfile_free(tmp);
+   return NULL;
}
}
-   return fd;
+
+   return tmp;
 }
 
 static void domain_close_tty(struct domain *dom)
@@ -665,8 +669,8 @@ static int domain_create_ring(struct domain *dom)
}
}
 
-   if (log_guest && (dom->log_fd == -1))
-   dom->log_fd = create_domain_log(dom);
+   if (log_guest && !dom->log)
+   dom->log = create_domain_log(dom);
 
  out:
return err;
@@ -724,7 +728,6 @@ static struct domain *create_domain(int domid)
dom->master_fd = -1;
dom->master_pollfd_idx = -1;
dom->slave_fd = -1;
-   dom->log_fd = -1;
dom->xce_pollfd_idx = 

[Xen-devel] [PATCH 1/6] xenconsoled: introduce log file abstraction

[Wei Liu]

It will be used to handle hypervsior log and guest console log.

Signed-off-by: Wei Liu 
---
 tools/console/daemon/logfile.c | 229 +
 tools/console/daemon/logfile.h |  57 ++
 2 files changed, 286 insertions(+)
 create mode 100644 tools/console/daemon/logfile.c
 create mode 100644 tools/console/daemon/logfile.h

diff --git a/tools/console/daemon/logfile.c b/tools/console/daemon/logfile.c
new file mode 100644
index 000..ad73f8e
--- /dev/null
+++ b/tools/console/daemon/logfile.c
@@ -0,0 +1,229 @@
+/*
+ *  Copyright (C) Citrix 2016
+ *  Author(s): Wei Liu 
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; under version 2 of the License.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; If not, see .
+ */
+#define _GNU_SOURCE
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "utils.h"
+#include "logfile.h"
+
+static void logfile_entry_free(struct logfile_entry *entry)
+{
+   if (!entry) return;
+
+   if (entry->fd >= 0) close(entry->fd);
+   free(entry);
+}
+
+void logfile_free(struct logfile *f)
+{
+   if (!f) return;
+
+   logfile_entry_free(f->entry);
+
+   free(f->basepath);
+   free(f);
+}
+
+static struct logfile_entry *logfile_entry_new(const char *path, mode_t mode)
+{
+   struct logfile_entry *entry;
+   struct stat sb;
+
+   entry = calloc(1, sizeof(*entry));
+   if (!entry) goto err;
+
+   entry->fd = open(path, O_CREAT|O_APPEND|O_WRONLY|O_CLOEXEC, mode);
+   if (entry->fd < 0) {
+   dolog(LOG_ERR, "Failed to open log file %s", path);
+   goto err;
+   }
+
+   entry->pos = lseek(entry->fd, 0, SEEK_END);
+   if (entry->pos == (off_t)-1) {
+   dolog(LOG_ERR, "Unable to determine current file offset in %s",
+ path);
+   goto err;
+   }
+
+   if (fstat(entry->fd, ) < 0) {
+   dolog(LOG_ERR, "Unable to fstat current file %s", path);
+   goto err;
+   }
+
+   entry->len = sb.st_size;
+   return entry;
+
+err:
+   logfile_entry_free(entry);
+   return NULL;
+}
+
+struct logfile *logfile_new(const char *path, mode_t mode)
+{
+   struct logfile *logfile;
+
+   logfile = calloc(1, sizeof(*logfile));
+   if (!logfile) goto err;
+
+   logfile->basepath = strdup(path);
+   if (!logfile->basepath) {
+   dolog(LOG_ERR, "Failed to strdup %s", path);
+   goto err;
+   }
+
+   logfile->mode = mode;
+   logfile->maxlen = LOGFILE_MAX_SIZE;
+
+
+   logfile->entry = logfile_entry_new(logfile->basepath, mode);
+
+   if (!logfile->entry) goto err;
+
+   return logfile;
+err:
+   logfile_free(logfile);
+   return NULL;
+}
+
+static int logfile_rollover(struct logfile *file)
+{
+   struct logfile_entry *old = file->entry, *new = NULL;
+   char *this = NULL;
+   char *next = NULL;
+   unsigned i;
+
+   if (asprintf(, "%s.%u", file->basepath,
+LOGFILE_MAX_BACKUP_NUM) < 0) {
+   dolog(LOG_ERR, "Failed to asprintf %s.%u",
+ file->basepath, LOGFILE_MAX_BACKUP_NUM);
+   goto err;
+   }
+
+   for (i = LOGFILE_MAX_BACKUP_NUM; i > 0; i--) {
+   if (i == 1) {
+   this = strdup(file->basepath);
+   if (!this) {
+   dolog(LOG_ERR, "Failed to strdup %s",
+ file->basepath);
+   goto err;
+   }
+   } else {
+   if (asprintf(, "%s.%u", file->basepath, i-1) < 0) {
+   dolog(LOG_ERR, "Failed to asprintf %s.%u",
+ file->basepath, i-1);
+   goto err;
+   }
+   }
+
+   if (rename(this, next) < 0 && errno != ENOENT) {
+   dolog(LOG_ERR, "Failed to rename %s to %s",
+ this, next);
+   goto err;
+   }
+
+   free(next);
+   next = this;
+   this = NULL;
+   }
+
+   new = logfile_entry_new(file->basepath, file->mode);
+   if (!new) goto err;
+
+   file->entry = new;
+   logfile_entry_free(old);
+
+   return 0;
+err:
+   free(this);
+   free(next);
+ 

Re: [Xen-devel] [PATCH v5 5/9] monitor: ARM SMC events

[Tamas K Lengyel]

On Mon, Jun 6, 2016 at 9:54 AM, Julien Grall  wrote:
> Hello Tamas,
>
> On 06/06/16 16:24, Tamas K Lengyel wrote:
>>
>>
>> On Jun 6, 2016 04:08, "Julien Grall" > > wrote:
>>  >
>>  > Hello,
>>  >
>>  >
>>  > On 04/06/2016 18:40, Tamas K Lengyel wrote:
>>  >>
>>  >> On Sat, Jun 4, 2016 at 3:03 AM, Edgar E. Iglesias
>>  >> > wrote:
>>  >>>
>>  >>> Forwarding SMC events for SMC insns that didn't pass the condition
>> tests
>>  >>> doesn't make any sense to me. It'll just make the receivers job
>> harder.
>>  >>> Why would a receiver want to do anything else than drop these?
>>  >>> If it actually does look at them it'll be looking at implementation
>>  >>> defined HW behaviour that may vary between CPU implementations.
>>  >>
>>  >>
>>  >> If for no other purposes it may be useful to log them to be able to
>>  >> study the CPU implementation's behavior.
>>  >
>>  >
>>  > I cannot see how you will be able to study ARM CPU implementation's
>> behavior with VM event. Though I am not familiar with it.
>>  >
>>  > For now, it looks like to me that forwarding conditional SMC even if
>> the condition check has failed will require a lot of code in each
>> introspection applications, not to mention that they will need specific
>> code to distinguish ARMv7 vs ARMv8.
>>
>> Why would it require any more code? Right now the only thing the
>> listener can do is to increment pc to jump over the SMC. That would be
>> the same regardless of what type it was. As for checking whether it was
>> v7 or v8, if that is of interest to the app it should implement the
>> appropriate logic for it. I don't see a problem there either.
>
>
> A listener can to do more then incrementing pc to jump over the SMC. The app
> may want to decode the instruction to get the immediate and then execute
> different actions depending on its value (for instance to emulate some SMC
> call).
>
> For this kind of app, it will be necessary to find out whether the condition
> check has failed or not before executing it.
>

Sure, and if that need arises the extra information can also be
forwarded. Especially if as you said you are planning to implement the
decoding for Xen to see if it was a failed condition check or not.

Tamas

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

Re: [Xen-devel] XSA-180 follow-up: repurpose xenconsoled for logging

[Wei Liu]

On Fri, Jun 03, 2016 at 05:57:29PM +0100, Ian Jackson wrote:
> Wei Liu writes ("XSA-180 follow-up: repurpose xenconsoled for logging"):
> > XXX DRAFT DRAFT DRAFT XXX
> 
> Thanks!
> 
> > Per domain logging via xenconsoled
> > ==
> > 
> > As of Xen release XXX, xenconsoled is repurposed to handle logging for
> > QEMU. Libxenlight will arrange xenconsoled to create and handle the
> > log file. It's possible to expose API(s) so that the user of
> > libxenlight can leverage such ability, but it is not currently done.
> > 
> > Xenstore path and nodes
> > ---
> > 
> > Libxenlight communicates with xenconsoled via a simple xenstore based
> > protocol.  All information for a specific domain is stored under
> > /libxl/$DOMID/logging. Each log file has its own unique id ($LOGFILEID).
> 
> Are these IDs short strings or what ?
> 

Numeric ID.

> > Several xenstore nodes are needed (placed under logging/$LOGFILEID).
> > 
> >   pipe: the absolute path of the logging pipe
> >   file: the absolute path of the file to write to
> >   limit: the maximum length of the file before it gets rotated
> >   copies: the number of copies to keep
> >   state: xenconsoled writes "ready" to this node to indicate readiness
> 
> The use of named pipes for rendezvous is a little unusual, but I think
> given that xenconsoled is primarily driven by xenstore it is probably
> appropriate.
> 
> > Xenconsoled will sanitise both pipe and file fields. Pipe has to be
> > placed under XEN_RUN_DIR. File has to be placed under /var/log/xen
> > (XXX doesn't seem to be configurable at the moment, should introduce
> > XEN_LOG_DIR?).
> 
> Yes.
> 
> NB the pipe has to be mode 600.  Also, opening a fifo has to be done
> with care.  For example, the following dance is needed get a
> write-only fd without risking blocking:
> 
>fdrw = open("/var/run/xen/DOMAIN.qemu.fifo", O_RDWR);
>fdwo = open("/var/run/xen/DOMAIN.qemu.fifo", O_WRONLY);
>close(fdwr);
> 

Heh, good trick.

> But I think qemu should be provided with a O_RDWR fd.  This is because
> otherwise, if xenconsoled crashes or is restarted or something, qemu
> would get a SIGPIPE (or EPIPE) from writes to its stderr.
> 
> It is better for a qemu in this situation to block waiting for a new
> xenconsoled to appear.
> 

So there are two issues:
1. xenconsoled crashes
2. xenconsoled gets bogged down, not able to process things fast enough

Either issue will render the client (QEMU for now) blocked.

I think we need to think this through and set the expectation straight
here.

> > 1. Libxenlight:
> >   1. Generates a unique log file id $LOGFILEID
> >   2. Creates a pipe $PIPE
> >   3. Writes parameter to xenstore
> >   4. Wait for readiness indication
> > 2. Xenconsoled
> >   1. Watch global logging and per domain logging xenstore paths
> >   2. Gets notified, read parameters from xenstore
> >   3. Sanitise parameters
> >   4. Create log files
> >   5. Connect to the pipe provided
> >   6. Write "ready" to xenstore state node
> 
> Is it actually necessary to synchronise ?  If xenconsoled is slow, the
> approach above will simply block qemu's writes once the pipe buffer is
> full, until xenconsoled catches up.
> 
> If xenconsoled is missing the lack of synchronisation will result in
> qemu blocking, perhaps tripping the domain startup qemu readiness
> timeout.
> 

I will think about this later. This is related to the issues I mentioned
above.

> > 3. Libxenlight:
> >   1. Detects ready state from xenconsoled
> >   2. Open the pipe and return relevant handles to user
> > 
> > In case of xenconsoled failure, libxenlight will time out and bail.
> 
> Would it be possible for a user to connect to xenconsoled and get a
> copy of the qemu output with a suitable `xl console' rune ?
> 

An xl console extension to attach a particular logging stream that is
handled by xenconsoled is doable, I think. It's yet another fd anyway.

But for the time being I don't think this is essential because user can
use existing system utilities (tail --follow=name for example)  to
achieve the same functionality.

> > Clean up
> > 
> > 
> > When doing per domain logging, libxenlight will remove all domain
> > specific xenstore paths when a guest is gone. Xenconsoled use that to
> > do clean up.
> > 
> > Libxenlight is responsible for deleting the pipe.
> 
> The logfiles will presumably remain.
> 
> > Global logging
> > --
> > 
> > Since we don't plan to provide new APIs now, we don't support global
> > logging because that would require us to provide a cleanup API that
> > libxenlight users can call.
> 
> I don't understand what you mean by `global logging'.
> 

Right, that's a very confusing term. Let me explain.

So with all the machinery in place, we can handle not only guest logs
but logs from all sorts of xen components. Say, xenstored wants to write
to a log file, no problem, just call libxl_create_logfile and you now
get a fd you can write to, and 

Re: [Xen-devel] XSA-180 follow-up: repurpose xenconsoled for logging

[Wei Liu]

On Mon, Jun 06, 2016 at 02:03:45PM +0100, Andrew Cooper wrote:
> On 06/06/16 11:12, George Dunlap wrote:
> > On 03/06/16 18:38, Andrew Cooper wrote:
> >> On 01/06/16 15:00, Wei Liu wrote:
> >>> Hi all
> >>>
> >>> During the discussion of XSA-180 Ian came up with the idea that we
> >>> repurpose xenconsoled to handle logging. I've done some research (and
> >>> some coding as well!).
> >>>
> >>> In my reply to George the other day:
> >>>
>  I just read the code of virtlogd and xenconsoled.
> 
>  I think xenconsoled is missing at least things.
> 
>  From a higher level:
> 
>  1. Abstraction of rotating file.
>  2. Abstraction of client.
>  3. IPC interface to libxl -- presumably we need to create a socket.
> 
> >>> I've done #1 and port existing code to use that -- would be useful in
> >>> general.
> >>>
> >>> #2 is not too hard because xenconsoled already has concept of a domain.
> >>> I suspect some refactoring will be fine.
> >>>
> >>> #3 requires a bit more thinking. Virtlogd has an RPC interface -- I'm
> >>> pretty sure we *don't* want that for xenconsoled. So I spent some time
> >>> this morning and write up a draft for a xenstore based protocol. See
> >>> below.
> >>>
> >>> Also there is an implication here: we put xenconsoled in a really
> >>> critical path. If for some reason it doesn't work all guests are
> >>> blocked. Do we really want to do this?
> >> Sorry to be late to this thread, but I think all my Xen 4.7 tasks are
> >> now done.
> >>
> >> Architecturally, this is a bad idea, and -1 from me.
> >>
> >> First of all, you are making the assumption that xenconsoled and all
> >> qemus are in the same domain.  This is not necessarily the case,
> >> although I suppose that `xl console` does depend on xl and xenconsoled
> >> being in the same domain.
> >>
> >> At the moment, if xenconsoled crashes, the worst that happens is that
> >> you cant interact with guest consoles, and the guests will notice that
> >> their console rings aren't being drained.  This is a safe, albeit
> >> suboptimal, situation which allows guests to continue running unimpeeded.
> >>
> >> If qemu is connected via xenconsoled, and xenconsoled crashes, qemu will
> >> block.  Once qemu blocks, the VM is for all intents and purposes, dead. 
> >> As you identify, there is a security implication here, where a guest
> >> which can crash xenconsoled can also DoS all other HVM domains in the
> >> system.
> >>
> >> From the scalability side, XenServer supports running 1000 windows VMs
> >> on a single host (subject to sufficient RAM), and xenstored is a big
> >> monlithic single point of failure.  We would like to avoid adding a second.
> > qemu stubdoms is a red herring; that's a separate codepath which will be
> > modified appropriately for its use case.
> 
> Most certainly not (and I didn't specify qemu stubdoms).  What happens
> if someone wishes to run xenconsoled in a separate non-stub domain? 
> What happens if someone has separate non-stub domains for running qemus?
> 
> All of these are currently viable options with XSM.
> 

The scalability issue is true. TBH I'm also a bit skeptical to the
scalability issue. But in the end I think it wouldn't be any different
if you want to use syslog or any other daemon. The issue is going to be
the there if you opt to use any daemon to handle log.

There is a difference between requiring QEMU and the logging daemon to
live in the same domain v.s.  requiring the toolstack that sets up
logging for QEMU knows how to do it. That is, the whatever toolstack to
spawn QEMU in the other domain will know how to handle logging anyway.
Running QEMU and xenconsoled in the same domain won't be a hard
requirement. It is fair to expect xenconsoled and the toolstack live in
the same domain (TOOLSTACK_DOMAIN), so it would be fair to expect
xenconsoled to be available when libxl tries to start QEMU.

I wouldn't expect xenconsoled to crash easily.

> > The potential for qemu to
> > block if the daemon dies, as well as scalability concerns, are good
> > points which should be considered; and we certainly need to allow other
> > ways of dealing with logging.
> >
> > But...
> >
> >> If someone builds Xen from source and finds later that their disks are
> >> full up with logs, then tough.
> > ...I cannot express how wrong-headed I think this statement is; and...
> 
> That is merely your opinion.  You know mine, and I stand by it.
> 

I don't think these two views are fundamentally in conflict with each
other. To provide a system level solution, we need to provide something
to be integrated with the system. There is none so far.

> >
> >> If someone installs Xen from a distro,
> >> they should reasonably expect logging, and log management, to be
> >> consistent with other packages from the same distro.
> > ...the degree to which this is true depends on the degree to which
> > distributions are actually equipped to deal with these sorts of things.
> 
> A distro which isn't 

Re: [Xen-devel] [PATCH RESEND 00/14] Xen ARM DomU ACPI support

[Julien Grall]

Hi Wei,

On 06/06/16 13:26, Wei Liu wrote:

On Tue, May 31, 2016 at 01:02:52PM +0800, Shannon Zhao wrote:

From: Shannon Zhao 

The design of this feature is described as below.
Firstly, the toolstack (libxl) generates the ACPI tables according the
number of vcpus and gic controller.

Then, it copies these ACPI tables to DomU memory space and passes
them to UEFI firmware through the "ARM multiboot" protocol.

At last, UEFI gets the ACPI tables through the "ARM multiboot" protocol
and installs these tables like the usual way and passes both ACPI and DT
information to the Xen DomU.

Currently libxl only generates RSDP, XSDT, GTDT, MADT, FADT, DSDT tables
since it's enough now.

This has been tested using guest kernel with the Dom0 ACPI support
patches which could be fetched from:
https://git.kernel.org/cgit/linux/kernel/git/mfleming/efi.git/log/?h=efi/arm-xen

Shannon Zhao (14):
   libxl/arm: Fix the function name in error log
   libxl/arm: Factor out codes for generating DTB
   libxc: Add placeholders for ACPI tables blob and size
   tools: add ACPI tables relevant definitions
   libxl/arm: Construct ACPI GTDT table
   libxl/arm: Construct ACPI FADT table
   libxl/arm: Construct ACPI DSDT table
   libxl/arm: Construct ACPI MADT table
   libxl/arm: Construct ACPI XSDT table
   libxl/arm: Construct ACPI RSDP table
   libxl/arm: Initialize domain param HVM_PARAM_CALLBACK_IRQ
   libxl/arm: Add ACPI module
   libxl/arm: initialize memory information of ACPI blob
   libxc/xc_dom_core: Copy ACPI tables to guest memory space



After going through this series, I think the code mostly looks good.

There is one higher level question: you seem to have put a lot of the
table construction code in libxl, while I failed to see why specific
libxl information is needed. Have you considered moving the code to
libxc?


I would rather avoid to have the device tree built by libxl and ACPI 
tables built by libxc.


I don't remember why we have decided to implement DT building in libxl, 
I guess it is because we need to know which GIC version is used (GICv2 
vs GICv3).


In the long run, we might also need to have more part of the firmware 
configurable (performance monitor support, memory layout, UART...).


Most of those information are available easily in libxl, we would to 
export them of libxc.


Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH RFC 14/20] acpi/hvmloader: Replace mem_alloc() and virt_to_phys() with memory ops

[Boris Ostrovsky]

On 06/06/2016 08:58 AM, Jan Beulich wrote:
 On 06.04.16 at 03:25,  wrote:
>> --- a/tools/firmware/hvmloader/acpi/acpi2_0.h
>> +++ b/tools/firmware/hvmloader/acpi/acpi2_0.h
>> @@ -490,6 +490,11 @@ struct acpi_numa {
>>  xen_vmemrange_t *vmemrange;
>>  };
>>  
>> +struct acpi_mem_ops {
>> +void *(*alloc)(uint32_t size, uint32_t align);
>> +unsigned long (*v2p)(void *v);
>> +};
> As before, I think this is the wrong header for such stuff.
>
> And then I think I agree with the allocation hook (or otherwise the
> too generic name would need to be changed), but I don't think I
> agree with the v2p one. Instead I think the consumer should
> provide a suitable virt_to_phys(): That's already the case for
> hvmloader, should be trivial for libxc. And as long as we provide
> Dom0 with a 1:1 machine/physical addresses, the existing one in
> the hypervisor would do too.
>
> At the very least alternatively, to reduce code churn, patch order
> should be changed, so that you don't touch again all the lines that
> you made use virt_to_phys() just a patch or two ago.

I'd rather go this route. IIRC I initially had both mem_alloc() and
virt_to_phys() being provided by callers by name but it ended up being
somewhat messy. And I think for consistency sake they should both be
either hooks or provided directly.

-boris


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

Re: [Xen-devel] [PATCH RESEND 00/14] Xen ARM DomU ACPI support

[Boris Ostrovsky]

On 06/06/2016 08:26 AM, Wei Liu wrote:
> On Tue, May 31, 2016 at 01:02:52PM +0800, Shannon Zhao wrote:
>> From: Shannon Zhao 
>>
>> The design of this feature is described as below.
>> Firstly, the toolstack (libxl) generates the ACPI tables according the
>> number of vcpus and gic controller.
>>
>> Then, it copies these ACPI tables to DomU memory space and passes
>> them to UEFI firmware through the "ARM multiboot" protocol.
>>
>> At last, UEFI gets the ACPI tables through the "ARM multiboot" protocol
>> and installs these tables like the usual way and passes both ACPI and DT
>> information to the Xen DomU.
>>
>> Currently libxl only generates RSDP, XSDT, GTDT, MADT, FADT, DSDT tables
>> since it's enough now.
>>
>> This has been tested using guest kernel with the Dom0 ACPI support
>> patches which could be fetched from:
>> https://git.kernel.org/cgit/linux/kernel/git/mfleming/efi.git/log/?h=efi/arm-xen
>>
>> Shannon Zhao (14):
>>   libxl/arm: Fix the function name in error log
>>   libxl/arm: Factor out codes for generating DTB
>>   libxc: Add placeholders for ACPI tables blob and size
>>   tools: add ACPI tables relevant definitions
>>   libxl/arm: Construct ACPI GTDT table
>>   libxl/arm: Construct ACPI FADT table
>>   libxl/arm: Construct ACPI DSDT table
>>   libxl/arm: Construct ACPI MADT table
>>   libxl/arm: Construct ACPI XSDT table
>>   libxl/arm: Construct ACPI RSDP table
>>   libxl/arm: Initialize domain param HVM_PARAM_CALLBACK_IRQ
>>   libxl/arm: Add ACPI module
>>   libxl/arm: initialize memory information of ACPI blob
>>   libxc/xc_dom_core: Copy ACPI tables to guest memory space
>>
> After going through this series, I think the code mostly looks good.
>
> There is one higher level question: you seem to have put a lot of the
> table construction code in libxl, while I failed to see why specific
> libxl information is needed. Have you considered moving the code to
> libxc?
>
> What I don't want to see is that x86 builds its ACPI table in libxc
> while ARM builds its in libxl. That would make future merger harder.
>
> Boris?

TBH, I am not sure which library this should really belong to. I felt
libxc would be more appropriate.

-boris


>
> (Please don't update your code before we come to a conclusion to avoid
> wasting your effort)
>
> Wei.



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

Re: [Xen-devel] [PATCH RESEND 05/14] libxl/arm: Construct ACPI GTDT table

[Julien Grall]

Hi Stefano,

On 06/06/16 15:50, Stefano Stabellini wrote:

On Mon, 6 Jun 2016, Shannon Zhao wrote:

I don't know why we need to disable ACPI because we can provide ACPI
tables but guest could choose to not use it. And for ARM32 domain, since
the linux guest kernel doesn't support ACPI, even we provide ACPI
tables, it can't use it, anyway.


Memory usage. Simplicity: if you know you are not going to use ACPI, you
might as well disable it to have one less moving piece (every line of
code is potential for a bug). Guest configuration: if your guest
operating system supports both ACPI and Device Tree and you want to be
sure that Device Tree is the one that gets used, then you can do it by
disabling ACPI at the VM level. Linux offers a command line option to do
that, but other OSes might not and could choose ACPI by default.


Other OSes would have the same problem on baremetal. A VM should 
reproduce the baremetal behavior. If you have enabled both ACPI and DT 
in your kernel, then you greed to let the kernel choose for you.


However, I agree that an having option to enable/disable ACPI is useful. 
The main use case would be embedded environment where ACPI will less used.


Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH RFC 12/20] acpi/hvmloader: Link ACPI object files directly

[Jan Beulich]

>>> On 06.06.16 at 17:31,  wrote:
> On 06/06/16 15:49, Boris Ostrovsky wrote:
>> On 06/06/2016 10:29 AM, Jan Beulich wrote:
 How would this help with avoiding building the intermediate files
 from multiple paces (libxc and hvmloader specifically --- that was
 the reason for .NOTPARALLEL). 
>>> If you symlink the source files into three different subdirectories,
>>> building in those three subdirectories will become independent. (Or,
>>> just like libelf, build the hypervisor part in the actual directory, and
>>> symlink only the libxc and hvmloader uses.)
>> I see. I didn't want to generate intermediate files multiple times but
>> yes, it's better than dealing with make synchronization issues.
> 
> It is exceedingly unlikely that Xen, the toolstack, and hvmloader will
> have libacpi compiled with the same compiler settings.
> 
> Sharing intermediate files is not safe to do in this context.

But you realize we don't talk about compiler settings here? The
intermediate .c files are to represent compiled .asl ones, and
the .asl -> .c conversion is much more likely to happen in mostly
similar fashion. But I can foresee customization, which would end
up being different between the consumer environments.

Jan


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

Re: [Xen-devel] [PATCH RESEND 05/14] libxl/arm: Construct ACPI GTDT table

[Julien Grall]

Hello,

On 06/06/16 13:04, Stefano Stabellini wrote:

On Mon, 6 Jun 2016, Julien Grall wrote:

On 06/06/16 12:40, Stefano Stabellini wrote:

On Tue, 31 May 2016, Shannon Zhao wrote:

From: Shannon Zhao 


ACPI tables for ARM guests should be user configurable: acpi=1 in the VM
config file enables them, with default off.


The VM system specification for ARM [1] mandates that both ACPI and DT should
be provided and described the entire VM and its peripheral (see the section
"Hardware Description").

Furthermore, the user may not know if the guest OS will use ACPI or DT For
instance Redhat is using ACPI whilst Debian is using DT.

So we have to provide both by default. However, 32-bit domain should only have
Device-Tree table created.

Anyway, the reason should have been described in the commit message. I would
split this patch in two: introducing prepare ACPI and then GTDT table. So we
can provide details in the commit message.


All right, let me rephrase then: we should have a VMSPEC=on or off to
enable or disable compliance with the VM system specification for ARM.
(The good thing about specifications is that there are so many to choose
from.) With compliance disabled, we can avoid introducing ACPI tables
for the guest.

Given that "VMSPEC" is cumbersome, I suggest to introduce a simpler and
more meaningful alias: "ACPI" :-)


The VM specification introduces other components such as a SBSA UART 
emulation (which is not yet implemented by Xen).


Do we want an option for each components?



I am open to discussion on what the default should be, but there needs
to be a way to disable ACPI.


Agree with that.

--
Julien Grall

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

Re: [Xen-devel] [PATCH RFC 12/20] acpi/hvmloader: Link ACPI object files directly

[Andrew Cooper]

On 06/06/16 15:49, Boris Ostrovsky wrote:
> On 06/06/2016 10:29 AM, Jan Beulich wrote:
>>> How would this help with avoiding building the intermediate files
>>> from multiple paces (libxc and hvmloader specifically --- that was
>>> the reason for .NOTPARALLEL). 
>> If you symlink the source files into three different subdirectories,
>> building in those three subdirectories will become independent. (Or,
>> just like libelf, build the hypervisor part in the actual directory, and
>> symlink only the libxc and hvmloader uses.)
> I see. I didn't want to generate intermediate files multiple times but
> yes, it's better than dealing with make synchronization issues.

It is exceedingly unlikely that Xen, the toolstack, and hvmloader will
have libacpi compiled with the same compiler settings.

Sharing intermediate files is not safe to do in this context.

~Andrew

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

Re: [Xen-devel] [PATCH v5 5/9] monitor: ARM SMC events

[Tamas K Lengyel]

On Jun 6, 2016 04:08, "Julien Grall"  wrote:
>
> Hello,
>
>
> On 04/06/2016 18:40, Tamas K Lengyel wrote:
>>
>> On Sat, Jun 4, 2016 at 3:03 AM, Edgar E. Iglesias
>>  wrote:
>>>
>>> Forwarding SMC events for SMC insns that didn't pass the condition tests
>>> doesn't make any sense to me. It'll just make the receivers job harder.
>>> Why would a receiver want to do anything else than drop these?
>>> If it actually does look at them it'll be looking at implementation
>>> defined HW behaviour that may vary between CPU implementations.
>>
>>
>> If for no other purposes it may be useful to log them to be able to
>> study the CPU implementation's behavior.
>
>
> I cannot see how you will be able to study ARM CPU implementation's
behavior with VM event. Though I am not familiar with it.
>
> For now, it looks like to me that forwarding conditional SMC even if the
condition check has failed will require a lot of code in each introspection
applications, not to mention that they will need specific code to
distinguish ARMv7 vs ARMv8.

Why would it require any more code? Right now the only thing the listener
can do is to increment pc to jump over the SMC. That would be the same
regardless of what type it was. As for checking whether it was v7 or v8, if
that is of interest to the app it should implement the appropriate logic
for it. I don't see a problem there either.

>
> Anyway, I am planning to send a patch to ignore conditional SMCs if the
condition check has failed because this is the right thing to do.

As I said I don't really care for these cases so fine by me.

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

Re: [Xen-devel] [PATCH RFC 13/20] acpi/hvmloader: Add stdio.h, string.h and x86.h

[Jan Beulich]

>>> On 06.06.16 at 17:08,  wrote:
> On 06/06/2016 07:31 AM, Jan Beulich wrote:
> On 06.04.16 at 03:25,  wrote:
>>> --- /dev/null
>>> +++ b/tools/firmware/hvmloader/acpi/x86.h
>>> @@ -0,0 +1,14 @@
>>> +#ifndef __ACPI_X86_H__
>>> +#define __ACPI_X86_H__
>>> +
>>> +#define IOAPIC_BASE_ADDRESS 0xfec0
>>> +#define IOAPIC_ID   0x01
>>> +#define IOAPIC_VERSION  0x11
>>> +
>>> +#define LAPIC_BASE_ADDRESS  0xfee0
>>> +#define LAPIC_ID(vcpu_id)   ((vcpu_id) * 2)
>>> +
>>> +#define PCI_ISA_DEVFN   0x08/* dev 1, fn 0 */
>>> +#define PCI_ISA_IRQ_MASK0x0c20U /* ISA IRQs 5,10,11 are PCI connected 
>>> */
>> Only the latter of those last two is used by build.c. Please limit what
>> you move to the very minimum. I.e. IOAPIC_VERSION shouldn't move
>> either imo (and I notice there's a disconnect between it and the
>> hypervisor's VIOAPIC_VERSION_ID, but that's a pre-existing issue).
> 
> You don't think logically all these should be in the same file?

Not really, no. IOAPIC_VERSION, as said, should go away.
Meanwhile I already have a patch for that (at once dropping
IOAPIC_BASE_ADDRESS too), albeit that conflicts with what
you're doing.

And the two PCI_ISA_ ones seem to belong together solely
because of their name. The IRQ mask is a simple number,
unrelated to any specific PCI device.

Jan


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

Re: [Xen-devel] [PATCH RFC 18/20] libxc/acpi: Build ACPI tables for HVMlite guests

[Boris Ostrovsky]

On 06/06/2016 08:03 AM, Wei Liu wrote:
>
>> diff --git a/tools/libxc/Makefile b/tools/libxc/Makefile
>> index 608404f..9569e54 100644
>> --- a/tools/libxc/Makefile
>> +++ b/tools/libxc/Makefile
>> @@ -79,6 +79,26 @@ GUEST_SRCS-y += $(ELF_SRCS-y)
>>  $(patsubst %.c,%.o,$(ELF_SRCS-y)): CFLAGS += -Wno-pointer-sign
>>  $(patsubst %.c,%.opic,$(ELF_SRCS-y)): CFLAGS += -Wno-pointer-sign
>>  
>> +ACPI_PATH = $(XEN_ROOT)/xen/common/libacpi
>> +vpath %.c $(ACPI_PATH)
>> +ACPI_FILES  = dsdt_anycpu.c dsdt_15cpu.c static_tables.c
>> +ACPI_FILES += dsdt_anycpu_qemu_xen.c dsdt_empty.c build.c
>> +ACPI_SRCS = $(patsubst %.c,$(ACPI_PATH)/%.c,$(ACPI_FILES))
>> +
>> +.NOTPARALLEL: $(ACPI_SRCS)
> Why is this needed?


To prevent building intermediate *.c files in parallel from multiple
places (libxc and hvmloader). But this will be removed per Jan's
suggestion not to create those files in libacpi and instead keep them in
libxc and hvmloader.


>
> +static int init_acpi_config(struct xc_dom_image *dom,
> +struct acpi_config *config)
> +{
> +xc_interface *xch = dom->xch;
> +uint32_t domid = dom->guest_domid;
> +xc_dominfo_t info;
> +int i, rc;
> +
> +memset(config, 0, sizeof(*config));
> +
> +config->dsdt_anycpu = config->dsdt_15cpu = dsdt_empty;
> +config->dsdt_anycpu_len = config->dsdt_15cpu_len = dsdt_empty_len;
> +
> +rc = xc_domain_getinfo(xch, domid, 1, );
> +if ( rc < 0 )
> +{
> +DOMPRINTF("%s: getdomaininfo failed (rc=%d)", __FUNCTION__, rc);
> +return rc;
> +}
> +
> +config->apic_mode = 1;
> +
> +if ( dom->nr_vnodes )
> +{
> +struct acpi_numa *numa = >numa;
> +
> +numa->vmemrange = calloc(dom->nr_vmemranges,
> + sizeof(*numa->vmemrange));
> +numa->vdistance = calloc(dom->nr_vnodes,
> + sizeof(*numa->vdistance));
> +numa->vcpu_to_vnode = calloc(config->nr_vcpus,
> + sizeof(*numa->vcpu_to_vnode));
> +if ( !numa->vmemrange || !numa->vdistance || !numa->vcpu_to_vnode )
> +{
> +DOMPRINTF("%s: Out of memory", __FUNCTION__);
> +free(numa->vmemrange);
> +free(numa->vdistance);
> +free(numa->vcpu_to_vnode);
> +return -ENOMEM;
> +}
> +
> +rc = xc_domain_getvnuma(xch, domid, >nr_vnodes,
> +>nr_vmemranges,
> +>nr_vcpus, numa->vmemrange,
> +numa->vdistance, numa->vcpu_to_vnode);
> +
> + if ( rc )
> +{
> +DOMPRINTF("%s: xc_domain_getvnuma failed (rc=%d)", __FUNCTION__, 
> rc);
> +return rc;
> +}
> +}
> +else
> +config->nr_vcpus = info.max_vcpu_id + 1;
> This looks wrong, at least it is not immediately clear why you would
> want to do this.


Why is this wrong? If we have one VCPU max_vcpu_id will be zero, won't it?


> +}
> +
> +int xc_dom_build_acpi(struct xc_dom_image *dom)
> +{
> +struct acpi_config config;
> +uint32_t domid = dom->guest_domid;
> +xc_interface *xch = dom->xch;
> +int rc, i, acpi_pages_num;
> +xen_pfn_t extent, *extents;
> +void *acpi_pages, *acpi_physical;
> +void *guest_info_page, *guest_acpi_pages;
> +
> Need to initialise these to NULL and 0 in case you take the exit path
> earlier.

Right. I think Roger pointed out that clang doesn't like this neither.

-boris



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

Re: [Xen-devel] [PATCH RFC 13/20] acpi/hvmloader: Add stdio.h, string.h and x86.h

[Boris Ostrovsky]

On 06/06/2016 07:31 AM, Jan Beulich wrote:
 On 06.04.16 at 03:25,  wrote:
>> Users of ACPI builder will need to provide their own implemetations of
>> strncpy(), memcpy, memset() and printf declared in stdio.h and string.h.
>> For hvmloader we provide those two as wrappers around utul.h.
>>
>> Some x86-specific definitions move to ACPI builder's x86.h file so that
>> users won't have to re-define them themselves.
>>
>> Explicitly define a few things that are currently available to to build.c
>> via util.h.
> Considering the first paragraph, this last sentence was more
> confusing than clarifying to me: I expected util.h to get changed
> in some way, but aiui having gone through the entire patch this is
> just a (very different) re-statement of what you've said already.
>
>>  create mode 100644 tools/firmware/hvmloader/acpi/x86.h
>>  create mode 100644 tools/firmware/hvmloader/stdio.h
>>  create mode 100644 tools/firmware/hvmloader/string.h
> I have to admit that I'm not really looking forward to see the
> hypervisor gain a file stdio.h. I'm also not convinced libxc'e
> planned use of the libacpi/ would fit well with uses of printf() in
> that subtree.

Hypervisor won't use it, it will be included under '#ifndef __XEN__' (in
patch 20).

>
>> --- /dev/null
>> +++ b/tools/firmware/hvmloader/acpi/x86.h
>> @@ -0,0 +1,14 @@
>> +#ifndef __ACPI_X86_H__
>> +#define __ACPI_X86_H__
>> +
>> +#define IOAPIC_BASE_ADDRESS 0xfec0
>> +#define IOAPIC_ID   0x01
>> +#define IOAPIC_VERSION  0x11
>> +
>> +#define LAPIC_BASE_ADDRESS  0xfee0
>> +#define LAPIC_ID(vcpu_id)   ((vcpu_id) * 2)
>> +
>> +#define PCI_ISA_DEVFN   0x08/* dev 1, fn 0 */
>> +#define PCI_ISA_IRQ_MASK0x0c20U /* ISA IRQs 5,10,11 are PCI connected */
> Only the latter of those last two is used by build.c. Please limit what
> you move to the very minimum. I.e. IOAPIC_VERSION shouldn't move
> either imo (and I notice there's a disconnect between it and the
> hypervisor's VIOAPIC_VERSION_ID, but that's a pre-existing issue).

You don't think logically all these should be in the same file?

-boris


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

Re: [Xen-devel] Xen 4.7 crash

[Aaron Cornelius]

On 6/6/2016 10:19 AM, Wei Liu wrote:

On Mon, Jun 06, 2016 at 03:05:47PM +0100, Julien Grall wrote:

(CC Ian, Stefano and Wei)

Hello Aaron,

On 06/06/16 14:58, Aaron Cornelius wrote:

On 6/2/2016 5:07 AM, Julien Grall wrote:

Hello Aaron,

On 02/06/2016 02:32, Aaron Cornelius wrote:

This is with a custom application, we use the libxl APIs to interact
with Xen.  Domains are created using the libxl_domain_create_new()
function, and domains are destroyed using the libxl_domain_destroy()
function.

The test in this case creates a domain, waits a minute, then
deletes/creates the next domain, waits a minute, and so on.  So I
wouldn't be surprised to see the VMID occasionally indicate there are 2
active domains since there could be one being created and one being
destroyed in a very short time.  However, I wouldn't expect to ever have
256 domains.


Your log has:

(XEN) grant_table.c:3288:d0v1 Grant release (0) ref:(9) flags:(2) dom:(0)
(XEN) grant_table.c:3288:d0v1 Grant release (1) ref:(11) flags:(2)
dom:(0)

Which suggest that some grants are still mapped in DOM0.



The CubieTruck only has 2GB of RAM, I allocate 512MB for dom0 which
means that only 48 of the the Mirage domains (with 32MB of RAM) would
work at the same time anyway.  Which doesn't account for the various
inter-domain resources or the RAM used by Xen itself.


All the pages who belongs to the domain could have been freed except the
one referenced by DOM0. So the footprint of this domain will be limited
at the time.

I would recommend you to check how many domain are running at this time
and if DOM0 effectively released all the resources.


If the p2m_teardown() function checked for NULL it would prevent the
crash, but I suspect Xen would be just as broken since all of my
resources have leaked away.  More broken in fact, since if the board
reboots at least the applications will restart and domains can be
recreated.

It certainly appears that some resources are leaking when domains are
deleted (possibly only on the ARM or ARM32 platforms).  We will try to
add some debug prints and see if we can discover exactly what is
going on.


The leakage could also happen from DOM0. FWIW, I have been able to cycle
2000 guests over the night on an ARM platforms.



We've done some more testing regarding this issue.  And further testing
shows that it doesn't matter if we delete the vchans before the domains
are deleted.  Those appear to be cleaned up correctly when the domain is
destroyed.

What does stop this issue from happening (using the same version of Xen
that the issue was detected on) is removing any non-standard xenstore
references before deleting the domain.  In this case our application
allocates permissions for created domains to non-standard xenstore
paths.  Making sure to remove those domain permissions before deleting
the domain prevents this issue from happening.


I am not sure to understand what you mean here. Could you give a quick
example?


So we have a custom xenstore path for our tool (/tool/custom/ for the 
sake of this example), and we then allow every domain created using this 
tool to read that path.  When the domain is created, the domain is 
explicitly given read permissions using xs_set_permissions().  More 
precisely we:

1. retrieve the current list of permissions with xs_get_permissions()
2. realloc the permissions list to increase it by 1
3. update the list of permissions to give the new domain read only access
4. then set the new permissions list with xs_set_permissions()

We saw errors logged because this list of permissions was getting 
prohibitively large, but this error did not appear to be directly 
connected to the Xen crash I submitted last week.  Or so we thought at 
the time.


We realized that we had forgotten to remove the domain from the 
permissions list when the domain is deleted (which would cause the error 
we saw).  The application was updated to remove the domain from the 
permissions list:

1. retrieve the permissions with xs_get_permissions()
2. find the domain ID that is being deleted
3. memmove() the remaining domains down by 1 to "delete" the old domain 
from the permissions list

4. update the permissions with xs_set_permissions()

After we made that change, a load test over the weekend confirmed that 
the Xen crash no longer happens.  We checked this morning first thing 
and confirmed that without this change the crash reliably occurs.



It does not appear to matter if we delete the standard domain xenstore
path (/local/domain/) since libxl handles removing this path when
the domain is destroyed.

Based on this I would guess that the xenstore is hanging onto the VMID.




This is a somewhat strange conclusion. I guess the root cause is still
unclear at this point.


We originally tested a fix that explicitly cleaned up the vchans 
(created to communicate with the domains) before the 
xen_domain_destroy() function is called and there was no change.  We 
have confirmed that the vchans do not appear 

Re: [Xen-devel] [PATCH RFC 12/20] acpi/hvmloader: Link ACPI object files directly

[Jan Beulich]

>>> On 06.06.16 at 16:49,  wrote:
> On 06/06/2016 10:29 AM, Jan Beulich wrote:
>>> How would this help with avoiding building the intermediate files
>>> from multiple paces (libxc and hvmloader specifically --- that was
>>> the reason for .NOTPARALLEL). 
>> If you symlink the source files into three different subdirectories,
>> building in those three subdirectories will become independent. (Or,
>> just like libelf, build the hypervisor part in the actual directory, and
>> symlink only the libxc and hvmloader uses.)
> 
> I see. I didn't want to generate intermediate files multiple times but
> yes, it's better than dealing with make synchronization issues.

Plus, in the end, there may be a need for slightly different build
settings (resulting in slightly different intermediate files).

Jan


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

Re: [Xen-devel] [PATCH RESEND 05/14] libxl/arm: Construct ACPI GTDT table

[Stefano Stabellini]

On Mon, 6 Jun 2016, Shannon Zhao wrote:
> On 2016年06月06日 20:04, Stefano Stabellini wrote:
> > On Mon, 6 Jun 2016, Julien Grall wrote:
> >> > Hello,
> >> > 
> >> > On 06/06/16 12:40, Stefano Stabellini wrote:
> >>> > > On Tue, 31 May 2016, Shannon Zhao wrote:
>  > > > From: Shannon Zhao 
>  > > > 
>  > > > Construct GTDT table with the interrupt information of timers.
>  > > > 
>  > > > Signed-off-by: Shannon Zhao 
>  > > > ---
>  > > >   tools/libxl/libxl_arm.c | 75
>  > > > -
>  > > >   1 file changed, 74 insertions(+), 1 deletion(-)
>  > > > 
>  > > > diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
>  > > > index 9e99159..0fb4f69 100644
>  > > > --- a/tools/libxl/libxl_arm.c
>  > > > +++ b/tools/libxl/libxl_arm.c
>  > > > @@ -3,6 +3,7 @@
>  > > >   #include "libxl_libfdt_compat.h"
>  > > > 
>  > > >   #include 
>  > > > +#include 
>  > > >   #include 
>  > > >   #include 
>  > > >   #include 
>  > > > @@ -880,13 +881,85 @@ out:
>  > > >   return rc;
>  > > >   }
>  > > > 
>  > > > +static void make_acpi_header(struct acpi_table_header *h, const 
>  > > > char
>  > > > *sig,
>  > > > + int len, uint8_t rev)
>  > > > +{
>  > > > +memcpy(>signature, sig, 4);
>  > > > +h->length = len;
>  > > > +h->revision = rev;
>  > > > +memcpy(h->oem_id, ACPI_BUILD_APPNAME6, 6);
>  > > > +memcpy(h->oem_table_id, ACPI_BUILD_APPNAME4, 4);
>  > > > +memcpy(h->oem_table_id + 4, sig, 4);
>  > > > +h->oem_revision = 1;
>  > > > +memcpy(h->asl_compiler_id, ACPI_BUILD_APPNAME4, 4);
>  > > > +h->asl_compiler_revision = 1;
>  > > > +h->checksum = 0;
>  > > > +}
>  > > > +
>  > > > +static void make_acpi_gtdt(libxl__gc *gc, struct xc_dom_image 
>  > > > *dom)
>  > > > +{
>  > > > +struct acpi_gtdt_descriptor *gtdt;
>  > > > +
>  > > > +gtdt = libxl__zalloc(gc, sizeof(*gtdt));
>  > > > +
>  > > > +gtdt->secure_el1_interrupt = GUEST_TIMER_PHYS_S_PPI;
>  > > > +gtdt->secure_el1_flags = (ACPI_LEVEL_SENSITIVE <<
>  > > > ACPI_GTDT_INTERRUPT_MODE)
>  > > > + |(ACPI_ACTIVE_LOW <<
>  > > > ACPI_GTDT_INTERRUPT_POLARITY);
>  > > > +gtdt->non_secure_el1_interrupt = GUEST_TIMER_PHYS_NS_PPI;
>  > > > +gtdt->non_secure_el1_flags =
>  > > > + (ACPI_LEVEL_SENSITIVE <<
>  > > > ACPI_GTDT_INTERRUPT_MODE)
>  > > > + |(ACPI_ACTIVE_LOW <<
>  > > > ACPI_GTDT_INTERRUPT_POLARITY);
>  > > > +gtdt->virtual_timer_interrupt = GUEST_TIMER_VIRT_PPI;
>  > > > +gtdt->virtual_timer_flags =
>  > > > + (ACPI_LEVEL_SENSITIVE <<
>  > > > ACPI_GTDT_INTERRUPT_MODE)
>  > > > + |(ACPI_ACTIVE_LOW <<
>  > > > ACPI_GTDT_INTERRUPT_POLARITY);
>  > > > +
>  > > > +make_acpi_header(>header, "GTDT", sizeof(*gtdt), 2);
>  > > > +
>  > > > +dom->acpitable_blob->gtdt.table = (void *)gtdt;
>  > > > +/* Align to 64bit. */
>  > > > +dom->acpitable_blob->gtdt.size = sizeof(*gtdt);
>  > > > +dom->acpitable_size += dom->acpitable_blob->gtdt.size;
>  > > > +}
>  > > > +
>  > > > +static int prepare_acpi(libxl__gc *gc, libxl_domain_build_info 
>  > > > *info,
>  > > > +libxl__domain_build_state *state,
>  > > > +struct xc_dom_image *dom)
>  > > > +{
>  > > > +const libxl_version_info *vers;
>  > > > +
>  > > > +/* convenience aliases */
>  > > > +xc_domain_configuration_t *xc_config = >config;
>  > > > +
>  > > > +vers = libxl_get_version_info(CTX);
>  > > > +if (vers == NULL)
>  > > > +return ERROR_FAIL;
>  > > > +
>  > > > +LOG(DEBUG, "constructing ACPI tables for Xen version %d.%d 
>  > > > guest",
>  > > > +vers->xen_version_major, vers->xen_version_minor);
>  > > > +
>  > > > +/* Alloc memory for ACPI blob placeholders. */
>  > > > +dom->acpitable_blob = libxl__zalloc(gc, sizeof(struct
>  > > > acpitable_blob));
>  > > > +dom->acpitable_size = 0;
>  > > > +
>  > > > +make_acpi_gtdt(gc, dom);
>  > > > +
>  > > > +return 0;
>  > > > +}
>  > > > +
>  > > >   int libxl__arch_domain_init_hw_description(libxl__gc *gc,
>  > > >  
>  > > > libxl_domain_build_info
>  > > > *info,
>  > > >  
>  > > > libxl__domain_build_state
>  > > > *state,
>  > > >

Re: [Xen-devel] [PATCH RFC 12/20] acpi/hvmloader: Link ACPI object files directly

[Boris Ostrovsky]

On 06/06/2016 10:29 AM, Jan Beulich wrote:
>> How would this help with avoiding building the intermediate files
>> from multiple paces (libxc and hvmloader specifically --- that was
>> the reason for .NOTPARALLEL). 
> If you symlink the source files into three different subdirectories,
> building in those three subdirectories will become independent. (Or,
> just like libelf, build the hypervisor part in the actual directory, and
> symlink only the libxc and hvmloader uses.)

I see. I didn't want to generate intermediate files multiple times but
yes, it's better than dealing with make synchronization issues.

-boris


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

Re: [Xen-devel] [PATCH RESEND 1/4] libs, gnttab, libxc: Interface for grant copy operation

[Wei Liu]

On Tue, May 31, 2016 at 06:44:55AM +0200, Paulina Szubarczyk wrote:
[...]
>  /*
>   * Grant Sharing Interface (allocating and granting pages to others)
>   */
> diff --git a/tools/libs/gnttab/libxengnttab.map 
> b/tools/libs/gnttab/libxengnttab.map
> index dc737ac..6a94102 100644
> --- a/tools/libs/gnttab/libxengnttab.map
> +++ b/tools/libs/gnttab/libxengnttab.map
> @@ -12,6 +12,8 @@ VERS_1.0 {
>  
>   xengnttab_unmap;
>  
> + xengnttab_copy_grant;
> + 
>   xengntshr_open;
>   xengntshr_close;
>  

This is not correct. We probably need to use a new version here.

I would think we need to make the version 1.1 now because we add a new
function to it.

Anyway, this is not hard to fix, don't let this block you. If you're
interested in knowing the details:

https://www.gnu.org/software/gnulib/manual/html_node/LD-Version-Scripts.html

I'm expecting you to post another version.

Wei.

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

Re: [Xen-devel] Issues with PCI-Passtrough (VT-d) in HVM with Xen 4.6

[Boris Ostrovsky]

On 06/06/2016 10:21 AM, Jan Beulich wrote:
 On 06.06.16 at 16:01,  wrote:
>> On 06/06/2016 04:41 AM, Jan Beulich wrote:
 - on the DomU - when I run that "test console" tool, the "lspci -xxx" 
 output is different from before/after
 not much, though, only one register(?)

 diff lspci.before-testconsole lspci.after-testconsole
 2c2
 < 00: cf 15 00 00 02 01 00 02 00 00 00 ff 00 40 00 00
 ---
> 00: cf 15 00 00 02 01 00 02 00 00 00 ff 00 00 00 00
>>> Okay, that's the Latency Timer field, and I think just like BARs we
>>> may need to permit restoring this field. However, please note that
>>> the reset being done behind the back of pciback really is the
>>> problem here: pciback assumes (for a reason, as you can certainly
>>> understand) that it has full control over config space of a passed
>>> through device.
>>>
>>> And actually the latency timer would, as a side effect of enabling
>>> bus mastering on the device (via the pci_set_master() call from
>>> command_write()) set the Latency Timer field properly, just that
>>> again pciback (and the rest of Dom0's PCI subsystem) thinks that
>>> bus mastering is already enabled on the device. So perhaps in
>>> permissive mode we should simply allow the latency timer field to
>>> be written, just like we allow writing various of the Command
>>> Register bits in that mode. Maintainers, what do you think?
>> Don't we currently allow it to be written unconditionally? It is no
>> different from accessing Cache Line Size.
> No, we don't. And no, someone must have thought differently.
> This is what we have right now:
>
>   {
>/* Any side effects of letting driver domain control cache line? */
>.offset= PCI_CACHE_LINE_SIZE,
>.size  = 1,
>.u.b.read  = xen_pcibk_read_config_byte,
>.u.b.write = xen_pcibk_write_config_byte,
>   },
>   {
>.offset= PCI_LATENCY_TIMER,
>.size  = 1,
>.u.b.read  = xen_pcibk_read_config_byte,
>   },
>
> I.e. whoever wrote this originally thought that writes to Latency
> Timer should be suppressed altogether, and there may be reasons
> to also suppress writes to Cache Line Size.
>
>> Or are you suggesting to make it stricter?
> Well, following the comment I indeed thought about making the
> Cache Line Size one more strict. Latency Timer can't possibly be
> made more strict (unless we also wanted to disallow reads). 

Oh, yes, I completely mis-read the chunk that you quoted above. Didn't
notice that timer doesn't have a write op.

> And
> it is, iirc, irrelevant for PCIe anyway, so allowing writes there
> might be an option, but as they ought to have no effect, it would
> seem kind of pointless.
>
> In general I think we should be rather conservative with allowing
> writes to any register.

Yes.

-boris

>
> Jan
>
>>> If we decide to go that route, I would then wonder whether
>>> Cache Line Size being unconditionally writable right now would
>>> also better be restricted to permissive mode.
>>
>> -boris
>
>



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

Re: [Xen-devel] [PATCH RESEND 13/14] libxl/arm: initialize memory information of ACPI blob

[Shannon Zhao]

On 2016年06月06日 19:40, Stefano Stabellini wrote:
> On Tue, 31 May 2016, Shannon Zhao wrote:
>> From: Shannon Zhao 
>>
>> Assign the guest memory space for ACPI tables and replace the reg in DT
>> with real values.
>>
>> Signed-off-by: Shannon Zhao 
>> ---
>>  tools/libxc/xc_dom_arm.c | 16 +++-
>>  tools/libxl/libxl_arm.c  | 40 
>>  2 files changed, 55 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
>> index 64a8b67..e21e3e9 100644
>> --- a/tools/libxc/xc_dom_arm.c
>> +++ b/tools/libxc/xc_dom_arm.c
>> @@ -383,9 +383,11 @@ static int meminit(struct xc_dom_image *dom)
>>  const uint64_t kernsize = kernend - kernbase;
>>  const uint64_t dtb_size = dom->devicetree_blob ?
>>  ROUNDUP(dom->devicetree_size, XC_PAGE_SHIFT) : 0;
>> +const uint64_t acpi_size = dom->acpitable_blob ?
>> +ROUNDUP(dom->acpitable_size, XC_PAGE_SHIFT) : 0;
>>  const uint64_t ramdisk_size = dom->ramdisk_blob ?
>>  ROUNDUP(dom->ramdisk_size, XC_PAGE_SHIFT) : 0;
>> -const uint64_t modsize = dtb_size + ramdisk_size;
>> +const uint64_t modsize = dtb_size + acpi_size + ramdisk_size;
>>  const uint64_t ram128mb = bankbase[0] + (128<<20);
>>  
>>  xen_pfn_t p2m_size;
>> @@ -500,6 +502,18 @@ static int meminit(struct xc_dom_image *dom)
>>  modbase += dtb_size;
>>  }
>>  
>> +if ( acpi_size )
>> +{
>> +dom->acpi_seg.vstart = modbase;
>> +dom->acpi_seg.vend = modbase + acpi_size;
>> +
>> +DOMPRINTF("%s: acpi: 0x%" PRIx64 " -> 0x%" PRIx64 "",
>> +  __FUNCTION__,
>> +  dom->acpi_seg.vstart, dom->acpi_seg.vend);
>> +
>> +modbase += dtb_size;
> 
> shouldn't this be 
> 
> modbase += acpi_size
> 
> ?
> 
right, will fix.

> 
>> +}
>> +
>>  return 0;
>>  }
>>  
>> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
>> index e7cb578..bf1eeea 100644
>> --- a/tools/libxl/libxl_arm.c
>> +++ b/tools/libxl/libxl_arm.c
>> @@ -230,6 +230,27 @@ static int fdt_property_regs(libxl__gc *gc, void *fdt,
>>  return fdt_property(fdt, "reg", regs, sizeof(regs));
>>  }
>>  
>> +static int fdt_property_inplace_regs(void *fdt, int nodeoffset,
>> + unsigned addr_cells, unsigned 
>> size_cells,
>> + unsigned num_regs, ...)
>> +{
>> +uint32_t regs[num_regs*(addr_cells+size_cells)];
>> +be32 *cells = [0];
>> +int i;
>> +va_list ap;
>> +uint64_t base, size;
>> +
>> +va_start(ap, num_regs);
>> +for (i = 0 ; i < num_regs; i++) {
>> +base = addr_cells ? va_arg(ap, uint64_t) : 0;
>> +size = size_cells ? va_arg(ap, uint64_t) : 0;
>> +set_range(, addr_cells, size_cells, base, size);
>> +}
>> +va_end(ap);
>> +
>> +return fdt_setprop_inplace(fdt, nodeoffset, "reg", regs, sizeof(regs));
>> +}
> 
> The va_arg stuff is a bit overkill. Also, can't you call
> finalise_one_memory_node instead?
> 
OK, thank you.

-- 
Shannon

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

Re: [Xen-devel] [PATCH 3/2] xen-pciback: drop rom_init()

[Boris Ostrovsky]

On 06/06/2016 09:54 AM, Jan Beulich wrote:
 On 06.06.16 at 15:09,  wrote:
>> On 06/06/2016 04:47 AM, Jan Beulich wrote:
>>> It's identical to bar_init() now.
>>>
>>> Signed-off-by: Jan Beulich 
>>> ---
>>> I'm sorry for this 3/2 - I only now noticed that this additional
>>> simplification is now possible.
>> I wonder whether we should also move content of read_dev_bar() into
>> bar_init(). Especially given that it's not really reading a BAR but
>> rather initializing the stashed value.
> I had considered that too, but then thought the splitting out of
> that logic could as well stay. If we were to do that, I'd in fact
> prefer merging patches 2 and 3 (plus this additional adjustment).

If you could do that it would be great. (Again, mostly because I think
the name is misleading and renaming it to something like dev_bar_init()
would also not be good since we already have bar_init()).

Thanks.
-boris



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

Re: [Xen-devel] [PATCH RESEND 05/14] libxl/arm: Construct ACPI GTDT table

[Shannon Zhao]

On 2016年06月06日 20:04, Stefano Stabellini wrote:
> On Mon, 6 Jun 2016, Julien Grall wrote:
>> > Hello,
>> > 
>> > On 06/06/16 12:40, Stefano Stabellini wrote:
>>> > > On Tue, 31 May 2016, Shannon Zhao wrote:
 > > > From: Shannon Zhao 
 > > > 
 > > > Construct GTDT table with the interrupt information of timers.
 > > > 
 > > > Signed-off-by: Shannon Zhao 
 > > > ---
 > > >   tools/libxl/libxl_arm.c | 75
 > > > -
 > > >   1 file changed, 74 insertions(+), 1 deletion(-)
 > > > 
 > > > diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
 > > > index 9e99159..0fb4f69 100644
 > > > --- a/tools/libxl/libxl_arm.c
 > > > +++ b/tools/libxl/libxl_arm.c
 > > > @@ -3,6 +3,7 @@
 > > >   #include "libxl_libfdt_compat.h"
 > > > 
 > > >   #include 
 > > > +#include 
 > > >   #include 
 > > >   #include 
 > > >   #include 
 > > > @@ -880,13 +881,85 @@ out:
 > > >   return rc;
 > > >   }
 > > > 
 > > > +static void make_acpi_header(struct acpi_table_header *h, const char
 > > > *sig,
 > > > + int len, uint8_t rev)
 > > > +{
 > > > +memcpy(>signature, sig, 4);
 > > > +h->length = len;
 > > > +h->revision = rev;
 > > > +memcpy(h->oem_id, ACPI_BUILD_APPNAME6, 6);
 > > > +memcpy(h->oem_table_id, ACPI_BUILD_APPNAME4, 4);
 > > > +memcpy(h->oem_table_id + 4, sig, 4);
 > > > +h->oem_revision = 1;
 > > > +memcpy(h->asl_compiler_id, ACPI_BUILD_APPNAME4, 4);
 > > > +h->asl_compiler_revision = 1;
 > > > +h->checksum = 0;
 > > > +}
 > > > +
 > > > +static void make_acpi_gtdt(libxl__gc *gc, struct xc_dom_image *dom)
 > > > +{
 > > > +struct acpi_gtdt_descriptor *gtdt;
 > > > +
 > > > +gtdt = libxl__zalloc(gc, sizeof(*gtdt));
 > > > +
 > > > +gtdt->secure_el1_interrupt = GUEST_TIMER_PHYS_S_PPI;
 > > > +gtdt->secure_el1_flags = (ACPI_LEVEL_SENSITIVE <<
 > > > ACPI_GTDT_INTERRUPT_MODE)
 > > > + |(ACPI_ACTIVE_LOW <<
 > > > ACPI_GTDT_INTERRUPT_POLARITY);
 > > > +gtdt->non_secure_el1_interrupt = GUEST_TIMER_PHYS_NS_PPI;
 > > > +gtdt->non_secure_el1_flags =
 > > > + (ACPI_LEVEL_SENSITIVE <<
 > > > ACPI_GTDT_INTERRUPT_MODE)
 > > > + |(ACPI_ACTIVE_LOW <<
 > > > ACPI_GTDT_INTERRUPT_POLARITY);
 > > > +gtdt->virtual_timer_interrupt = GUEST_TIMER_VIRT_PPI;
 > > > +gtdt->virtual_timer_flags =
 > > > + (ACPI_LEVEL_SENSITIVE <<
 > > > ACPI_GTDT_INTERRUPT_MODE)
 > > > + |(ACPI_ACTIVE_LOW <<
 > > > ACPI_GTDT_INTERRUPT_POLARITY);
 > > > +
 > > > +make_acpi_header(>header, "GTDT", sizeof(*gtdt), 2);
 > > > +
 > > > +dom->acpitable_blob->gtdt.table = (void *)gtdt;
 > > > +/* Align to 64bit. */
 > > > +dom->acpitable_blob->gtdt.size = sizeof(*gtdt);
 > > > +dom->acpitable_size += dom->acpitable_blob->gtdt.size;
 > > > +}
 > > > +
 > > > +static int prepare_acpi(libxl__gc *gc, libxl_domain_build_info 
 > > > *info,
 > > > +libxl__domain_build_state *state,
 > > > +struct xc_dom_image *dom)
 > > > +{
 > > > +const libxl_version_info *vers;
 > > > +
 > > > +/* convenience aliases */
 > > > +xc_domain_configuration_t *xc_config = >config;
 > > > +
 > > > +vers = libxl_get_version_info(CTX);
 > > > +if (vers == NULL)
 > > > +return ERROR_FAIL;
 > > > +
 > > > +LOG(DEBUG, "constructing ACPI tables for Xen version %d.%d 
 > > > guest",
 > > > +vers->xen_version_major, vers->xen_version_minor);
 > > > +
 > > > +/* Alloc memory for ACPI blob placeholders. */
 > > > +dom->acpitable_blob = libxl__zalloc(gc, sizeof(struct
 > > > acpitable_blob));
 > > > +dom->acpitable_size = 0;
 > > > +
 > > > +make_acpi_gtdt(gc, dom);
 > > > +
 > > > +return 0;
 > > > +}
 > > > +
 > > >   int libxl__arch_domain_init_hw_description(libxl__gc *gc,
 > > >  libxl_domain_build_info
 > > > *info,
 > > >  
 > > > libxl__domain_build_state
 > > > *state,
 > > >  struct xc_dom_image 
 > > > *dom)
 > > >   {
 > > > +int rc;
 > > > +
 > > >   assert(info->type == LIBXL_DOMAIN_TYPE_PV);
 > > > -return prepare_dtb(gc, info, state, dom);
 > > > +rc = prepare_dtb(gc, info, state, dom);
 > > > +if (rc)
 > > > +

Re: [Xen-devel] [PATCH 1/2] xen-pciback: return proper values during BAR sizing

[Boris Ostrovsky]

On 06/06/2016 09:51 AM, Jan Beulich wrote:
 On 06.06.16 at 15:03,  wrote:
>> On 06/06/2016 04:11 AM, Jan Beulich wrote:
>>> @@ -225,38 +225,42 @@ static inline void read_dev_bar(struct p
>>>(PCI_BASE_ADDRESS_SPACE_MEMORY |
>>> PCI_BASE_ADDRESS_MEM_TYPE_64))) {
>>> bar_info->val = res[pos - 1].start >> 32;
>>> -   bar_info->len_val = res[pos - 1].end >> 32;
>>> +   bar_info->len_val = -resource_size([pos - 1]) >> 32;
>>> return;
>>> }
>>> }
>>>  
>>> +   if (!res[pos].flags ||
>>> +   (res[pos].flags & (IORESOURCE_DISABLED | IORESOURCE_UNSET |
>>> +  IORESOURCE_BUSY)))
>>> +   return;
>> Why are you not making this check first thing in the routine?
> For one, pos isn't set there yet. And I'd also rather avoid the
> complications resulting from 64-bit memory resources spanning
> two entries.

I thought that both words of a 64-bit BAR would result in a return under
this check so both would be zero. But yes, pos needs to be initialized
anyway (I didn't see this in the diff).

Reviewed-by: Boris Ostrovsky 


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

Re: [Xen-devel] [PATCH RFC 12/20] acpi/hvmloader: Link ACPI object files directly

[Jan Beulich]

>>> On 06.06.16 at 16:20,  wrote:
> On 06/06/2016 07:04 AM, Jan Beulich wrote:
>>
>>> --- a/tools/firmware/hvmloader/Makefile
>>> +++ b/tools/firmware/hvmloader/Makefile
>>> @@ -20,8 +20,6 @@
>>>  XEN_ROOT = $(CURDIR)/../../..
>>>  include $(XEN_ROOT)/tools/firmware/Rules.mk
>>>  
>>> -SUBDIRS := acpi
>>> -
>>>  # The HVM loader is started in 32-bit mode at the address below:
>>>  LOADADDR = 0x10
>>>  
>>> @@ -33,10 +31,18 @@ CFLAGS += $(CFLAGS_xeninclude)
>>>  # We mustn't use tools-only public interfaces.
>>>  CFLAGS += -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__
>>>  
>>> +# ACPI table builder
>>> +ACPI_PATH = $(XEN_ROOT)/tools/firmware/hvmloader/acpi
>>> +vpath %.c $(ACPI_PATH)
>> Please don't - this would preclude having files with the same names
>> in hvmloader/ and (the future) libacpi/.
>>
>>> @@ -95,7 +101,11 @@ all: subdirs-all
>>>  ovmf.o rombios.o seabios.o hvmloader.o: roms.inc
>>>  smbios.o: CFLAGS += -D__SMBIOS_DATE__="\"$(SMBIOS_REL_DATE)\""
>>>  
>>> -hvmloader: $(OBJS) acpi/acpi.a
>>> +.NOTPARALLEL: $(ACPI_SRC)
>> And very clearly we will want to avoid the use of .NOTPARALLEL
>> anywhere in our trees.
>>
>>> +$(ACPI_SRC):
>>> +   $(MAKE) -C $(ACPI_PATH)
>> The more that you can't do this from multiple points in the tree. Did
>> you consider e.g. symlinking the subtree into the multiple parent
>> trees?
> 
> How would this help with avoiding building the intermediate files from
> multiple paces (libxc and hvmloader specifically --- that was the reason
> for .NOTPARALLEL).

If you symlink the source files into three different subdirectories,
building in those three subdirectories will become independent. (Or,
just like libelf, build the hypervisor part in the actual directory, and
symlink only the libxc and hvmloader uses.)

> I thought of flock-ing directory but then I was afraid that interrupted
> make may not clean up properly.
> 
> Or are you suggesting symlinking to help with include paths (i.e. to
> drop vpath above)?

That as well - symlinking would solve a number of issues that I have
pointed out, afaict at least.

Jan


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

[Xen-devel] Xen Security Advisory 181 (CVE-2016-5242) - arm: Host crash caused by VMID exhaustion

[Xen . org security team]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Xen Security Advisory CVE-2016-5242 / XSA-181
  version 2

   arm: Host crash caused by VMID exhaustion

UPDATES IN VERSION 2


CVE assigned.

ISSUE DESCRIPTION
=

VMIDs are a finite hardware resource, and allocated as part of domain
creation.  If no free VMIDs are available when trying to create a new domain,
a bug in the error path causes a NULL pointer to be used, resulting in a Data
Abort and host crash.

IMPACT
==

Attempting to create too many concurrent domains causes a host crash rather
than a graceful error.  A malicious device driver domain can hold references
to domains, preventing its VMID being released.

VULNERABLE SYSTEMS
==

Xen versions 4.4 and later are affected.  Older Xen versions are unaffected.

x86 systems are not affected.

Only arm systems with less-privileged device driver domains can expose this
vulnerability.

MITIGATION
==

There is no mitigation.  Not using driver domains reclassifies the problem,
but does not fix it.

NOTE REGARDING LACK OF EMBARGO
==

The crash was discussed publicly on xen-devel, before it was appreciated
that there was a security problem.

CREDITS
===

This issue was discovered by Aaron Cornelius of DornerWorks.

RESOLUTION
==

Applying the appropriate attached patch resolves this issue.

xsa181.patch   xen-unstable, Xen 4.6.x, 4.5.x
xsa181-4.4.patch   Xen 4.4.x

$ sha256sum xsa181*
6756fcf6675e5277f6d6c0e8a0aaa51a7909ad9a55af89a09367fded8733  xsa181.patch
97a90c7cb42466647622cb2ed98de531b7ba2e174a1bc639a32a6f1b626d503f  
xsa181-4.4.patch
$
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.12 (GNU/Linux)

iQEbBAEBAgAGBQJXUYxcAAoJEIP+FMlX6CvZgAAH+OiNDLSkAHUl3isXjFzK+Mf9
NGuIyXc2j5K8uTwz5KvZkhiWLVCeOY7Jo1Wix3Fa1wFtJ2rMlgQf7/hOt0tk0NjU
w97Re+xSi69iruPEdwb4k31ohnlfLSqriqL4JWh6EDrhftdnvEk/yXmriyu1RhKy
MLk1P24Ora/gvSj31px3vBkbu8KLImhIOkOcRmJ7FQb8gWsmMDluuVu7lhUAL7im
KCe6u99sDQo18wxubYID4XxFqJExBUd6L3cnpdN4UITgylSaIqJq/RBwd8jRrxW8
MxT9/IcNf0rmB1Sh1IARBFF7P7hj76ho3sIpMeE0cMPWBe2NWMItX9ula61vQA==
=kBFB
-END PGP SIGNATURE-


xsa181.patch
Description: Binary data


xsa181-4.4.patch
Description: Binary data
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] Issues with PCI-Passtrough (VT-d) in HVM with Xen 4.6

[Jan Beulich]

>>> On 06.06.16 at 16:01,  wrote:
> On 06/06/2016 04:41 AM, Jan Beulich wrote:
>>
>>> - on the DomU - when I run that "test console" tool, the "lspci -xxx" 
>>> output is different from before/after
>>> not much, though, only one register(?)
>>>
>>> diff lspci.before-testconsole lspci.after-testconsole
>>> 2c2
>>> < 00: cf 15 00 00 02 01 00 02 00 00 00 ff 00 40 00 00
>>> ---
 00: cf 15 00 00 02 01 00 02 00 00 00 ff 00 00 00 00
>> Okay, that's the Latency Timer field, and I think just like BARs we
>> may need to permit restoring this field. However, please note that
>> the reset being done behind the back of pciback really is the
>> problem here: pciback assumes (for a reason, as you can certainly
>> understand) that it has full control over config space of a passed
>> through device.
>>
>> And actually the latency timer would, as a side effect of enabling
>> bus mastering on the device (via the pci_set_master() call from
>> command_write()) set the Latency Timer field properly, just that
>> again pciback (and the rest of Dom0's PCI subsystem) thinks that
>> bus mastering is already enabled on the device. So perhaps in
>> permissive mode we should simply allow the latency timer field to
>> be written, just like we allow writing various of the Command
>> Register bits in that mode. Maintainers, what do you think?
> 
> Don't we currently allow it to be written unconditionally? It is no
> different from accessing Cache Line Size.

No, we don't. And no, someone must have thought differently.
This is what we have right now:

{
 /* Any side effects of letting driver domain control cache line? */
 .offset= PCI_CACHE_LINE_SIZE,
 .size  = 1,
 .u.b.read  = xen_pcibk_read_config_byte,
 .u.b.write = xen_pcibk_write_config_byte,
},
{
 .offset= PCI_LATENCY_TIMER,
 .size  = 1,
 .u.b.read  = xen_pcibk_read_config_byte,
},

I.e. whoever wrote this originally thought that writes to Latency
Timer should be suppressed altogether, and there may be reasons
to also suppress writes to Cache Line Size.

> Or are you suggesting to make it stricter?

Well, following the comment I indeed thought about making the
Cache Line Size one more strict. Latency Timer can't possibly be
made more strict (unless we also wanted to disallow reads). And
it is, iirc, irrelevant for PCIe anyway, so allowing writes there
might be an option, but as they ought to have no effect, it would
seem kind of pointless.

In general I think we should be rather conservative with allowing
writes to any register.

Jan

>> If we decide to go that route, I would then wonder whether
>> Cache Line Size being unconditionally writable right now would
>> also better be restricted to permissive mode.
> 
> 
> -boris




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

Re: [Xen-devel] [PATCH RESEND 03/14] libxc: Add placeholders for ACPI tables blob and size

[Shannon Zhao]

On 2016年06月06日 20:16, Wei Liu wrote:
> On Mon, Jun 06, 2016 at 11:00:37AM +0100, Stefano Stabellini wrote:
>> > On Tue, 31 May 2016, Shannon Zhao wrote:
>>> > > From: Shannon Zhao 
>>> > > 
>>> > > Currently it only needs ACPI table RSDP, XSDT, GTDT, MADT, FADT, DSDT
>>> > > for ARM VM. So only add placeholders for them here.
>>> > > 
>>> > > Signed-off-by: Shannon Zhao 
>> > 
>> > If we are going to make this ARM only, then maybe we should consider
>> > moving these structs to an ARM specific header?
>> > 
> Agreed. Or at least have some ifdefs around the fields.
> 
Ok, will change. Thanks.

-- 
Shannon

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

Re: [Xen-devel] [PATCH RFC 12/20] acpi/hvmloader: Link ACPI object files directly

[Boris Ostrovsky]

On 06/06/2016 07:04 AM, Jan Beulich wrote:
>
>> --- a/tools/firmware/hvmloader/Makefile
>> +++ b/tools/firmware/hvmloader/Makefile
>> @@ -20,8 +20,6 @@
>>  XEN_ROOT = $(CURDIR)/../../..
>>  include $(XEN_ROOT)/tools/firmware/Rules.mk
>>  
>> -SUBDIRS := acpi
>> -
>>  # The HVM loader is started in 32-bit mode at the address below:
>>  LOADADDR = 0x10
>>  
>> @@ -33,10 +31,18 @@ CFLAGS += $(CFLAGS_xeninclude)
>>  # We mustn't use tools-only public interfaces.
>>  CFLAGS += -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__
>>  
>> +# ACPI table builder
>> +ACPI_PATH = $(XEN_ROOT)/tools/firmware/hvmloader/acpi
>> +vpath %.c $(ACPI_PATH)
> Please don't - this would preclude having files with the same names
> in hvmloader/ and (the future) libacpi/.
>
>> @@ -95,7 +101,11 @@ all: subdirs-all
>>  ovmf.o rombios.o seabios.o hvmloader.o: roms.inc
>>  smbios.o: CFLAGS += -D__SMBIOS_DATE__="\"$(SMBIOS_REL_DATE)\""
>>  
>> -hvmloader: $(OBJS) acpi/acpi.a
>> +.NOTPARALLEL: $(ACPI_SRC)
> And very clearly we will want to avoid the use of .NOTPARALLEL
> anywhere in our trees.
>
>> +$(ACPI_SRC):
>> +$(MAKE) -C $(ACPI_PATH)
> The more that you can't do this from multiple points in the tree. Did
> you consider e.g. symlinking the subtree into the multiple parent
> trees?

How would this help with avoiding building the intermediate files from
multiple paces (libxc and hvmloader specifically --- that was the reason
for .NOTPARALLEL).

I thought of flock-ing directory but then I was afraid that interrupted
make may not clean up properly.

Or are you suggesting symlinking to help with include paths (i.e. to
drop vpath above)?


-boris


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

Re: [Xen-devel] [PATCH RESEND 04/14] tools: add ACPI tables relevant definitions

[Shannon Zhao]

On 2016年06月06日 20:16, Wei Liu wrote:
> On Mon, Jun 06, 2016 at 06:27:25PM +0800, Shannon Zhao wrote:
>> > 
>> > 
>> > On 2016/6/6 18:11, Julien Grall wrote:
>>> > > Hi Stefano,
>>> > > 
>>> > > On 06/06/2016 11:04, Stefano Stabellini wrote:
 > >> On Tue, 31 May 2016, Shannon Zhao wrote:
> > >>> From: Shannon Zhao 
> > >>>
> > >>> Add ACPI tables relevant definitions for generating ACPI tables for 
> > >>> ARM
> > >>> guest later. Currently RSDP, XSDT, GTDT, MADT, FADT and DSDT tables 
> > >>> will
> > >>> be used.
> > >>>
> > >>> Signed-off-by: Shannon Zhao 
> > >>> ---
> > >>>  tools/libxc/include/acpi_defs.h | 277
> > >>> 
> > >>>  1 file changed, 277 insertions(+)
> > >>>  create mode 100644 tools/libxc/include/acpi_defs.h
> > >>>
> > >>> diff --git a/tools/libxc/include/acpi_defs.h
> > >>> b/tools/libxc/include/acpi_defs.h
> > >>> new file mode 100644
> > >>> index 000..9389a96
> > >>> --- /dev/null
> > >>> +++ b/tools/libxc/include/acpi_defs.h
> > >>> @@ -0,0 +1,277 @@
> > >>> +#ifndef _ACPI_DEFS_H_
> > >>> +#define _ACPI_DEFS_H_
> > >>> +
> > >>> +#define ACPI_BUILD_APPNAME6 "XenARM"
> > >>> +#define ACPI_BUILD_APPNAME4 "Xen "
 > >>
 > >> This header is actually ARM specific (also see the gic stuff below). 
 > >> It
 > >> is probably best to rename it to acpi_arm_defs.h.
>>> > > 
>>> > > Should not just we re-use the ACPI headers from xen/include/acpi/ ?
>> > So how to include those headers in tools/libxl/libxl_arm.c ?
>> > 
> Is it public headers?
no, it's not public.

> If so, it might already be available in
> tools/include. If not, is it feasible to be made public?
> 
I'm not sure it's ok to make ACPI defs public since there are not xen
specific.

> If in the end the file you need can't end up as a public header, you
> need to manipulate CFLAGS. There is one example in libxc's Makefile.
> Search for libelf.
> 
> But please make sure the CFLAGS doesn't get modified when it is not
> necessary.  I would expect the CFLAGS is explicitly altered for a list
> of files.
Thanks, I'll have a look.

-- 
Shannon

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

Re: [Xen-devel] Xen 4.7 crash

[Wei Liu]

On Mon, Jun 06, 2016 at 03:05:47PM +0100, Julien Grall wrote:
> (CC Ian, Stefano and Wei)
> 
> Hello Aaron,
> 
> On 06/06/16 14:58, Aaron Cornelius wrote:
> >On 6/2/2016 5:07 AM, Julien Grall wrote:
> >>Hello Aaron,
> >>
> >>On 02/06/2016 02:32, Aaron Cornelius wrote:
> >>>This is with a custom application, we use the libxl APIs to interact
> >>>with Xen.  Domains are created using the libxl_domain_create_new()
> >>>function, and domains are destroyed using the libxl_domain_destroy()
> >>>function.
> >>>
> >>>The test in this case creates a domain, waits a minute, then
> >>>deletes/creates the next domain, waits a minute, and so on.  So I
> >>>wouldn't be surprised to see the VMID occasionally indicate there are 2
> >>>active domains since there could be one being created and one being
> >>>destroyed in a very short time.  However, I wouldn't expect to ever have
> >>>256 domains.
> >>
> >>Your log has:
> >>
> >>(XEN) grant_table.c:3288:d0v1 Grant release (0) ref:(9) flags:(2) dom:(0)
> >>(XEN) grant_table.c:3288:d0v1 Grant release (1) ref:(11) flags:(2)
> >>dom:(0)
> >>
> >>Which suggest that some grants are still mapped in DOM0.
> >>
> >>>
> >>>The CubieTruck only has 2GB of RAM, I allocate 512MB for dom0 which
> >>>means that only 48 of the the Mirage domains (with 32MB of RAM) would
> >>>work at the same time anyway.  Which doesn't account for the various
> >>>inter-domain resources or the RAM used by Xen itself.
> >>
> >>All the pages who belongs to the domain could have been freed except the
> >>one referenced by DOM0. So the footprint of this domain will be limited
> >>at the time.
> >>
> >>I would recommend you to check how many domain are running at this time
> >>and if DOM0 effectively released all the resources.
> >>
> >>>If the p2m_teardown() function checked for NULL it would prevent the
> >>>crash, but I suspect Xen would be just as broken since all of my
> >>>resources have leaked away.  More broken in fact, since if the board
> >>>reboots at least the applications will restart and domains can be
> >>>recreated.
> >>>
> >>>It certainly appears that some resources are leaking when domains are
> >>>deleted (possibly only on the ARM or ARM32 platforms).  We will try to
> >>>add some debug prints and see if we can discover exactly what is
> >>>going on.
> >>
> >>The leakage could also happen from DOM0. FWIW, I have been able to cycle
> >>2000 guests over the night on an ARM platforms.
> >>
> >
> >We've done some more testing regarding this issue.  And further testing
> >shows that it doesn't matter if we delete the vchans before the domains
> >are deleted.  Those appear to be cleaned up correctly when the domain is
> >destroyed.
> >
> >What does stop this issue from happening (using the same version of Xen
> >that the issue was detected on) is removing any non-standard xenstore
> >references before deleting the domain.  In this case our application
> >allocates permissions for created domains to non-standard xenstore
> >paths.  Making sure to remove those domain permissions before deleting
> >the domain prevents this issue from happening.
> 
> I am not sure to understand what you mean here. Could you give a quick
> example?
> 
> >
> >It does not appear to matter if we delete the standard domain xenstore
> >path (/local/domain/) since libxl handles removing this path when
> >the domain is destroyed.
> >
> >Based on this I would guess that the xenstore is hanging onto the VMID.
> 

This is a somewhat strange conclusion. I guess the root cause is still
unclear at this point.

Is it possible that something else what rely on those xenstore node to
free up resources?

Wei.

> Regards,
> 
> -- 
> Julien Grall

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

Re: [Xen-devel] [RFC 10/16] xen/arm: Introduce alternative runtime patching

[Julien Grall]

Hi Konrad,

On 06/06/16 15:17, Konrad Rzeszutek Wilk wrote:

On Thu, Jun 02, 2016 at 04:14:04PM +0100, Julien Grall wrote:



On 02/06/16 16:04, Julien Grall wrote:

Hi Konrad,

On 02/06/16 15:46, Konrad Rzeszutek Wilk wrote:

On Tue, May 31, 2016 at 11:24:10AM +0100, Julien Grall wrote:

On 31/05/16 10:21, Stefano Stabellini wrote:

On Mon, 30 May 2016, Julien Grall wrote:

By spinning in __apply_alternatives_multi_stop we protect us against
modification in the common code and tricky bug (yes it might be
difficult to
hit and debug).


I feel that you are moving down the stack to try to make the
impact of doing modifications have no impact (or as low as possible).

And it would work now, but I am concerned that in the future it may be
not soo future proof.


Can you detail here?


Would it perhaps make sense to make some of the livepatching mechanism
be exposed as general code? And use it for alternative asm as well?


The code to sync the CPU looks very similar to stop_machine, so why
would we want to get yet another mechanism to sync the CPUs and execute
a specific function?


I forgot to mention that apply_alternatives_all is only called one time
during the boot and before CPU0 goes in idle_loop. If we have to patch
afterward, we would use apply_alternatives which consider that all the CPUs
have been parked somewhere else.


Oh, in that case please just mention that in the commit description.


Will do in the next version.

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [RFC 10/16] xen/arm: Introduce alternative runtime patching

[Konrad Rzeszutek Wilk]

On Thu, Jun 02, 2016 at 04:14:04PM +0100, Julien Grall wrote:
> 
> 
> On 02/06/16 16:04, Julien Grall wrote:
> >Hi Konrad,
> >
> >On 02/06/16 15:46, Konrad Rzeszutek Wilk wrote:
> >>On Tue, May 31, 2016 at 11:24:10AM +0100, Julien Grall wrote:
> >>>On 31/05/16 10:21, Stefano Stabellini wrote:
> On Mon, 30 May 2016, Julien Grall wrote:
> >>>By spinning in __apply_alternatives_multi_stop we protect us against
> >>>modification in the common code and tricky bug (yes it might be
> >>>difficult to
> >>>hit and debug).
> >>
> >>I feel that you are moving down the stack to try to make the
> >>impact of doing modifications have no impact (or as low as possible).
> >>
> >>And it would work now, but I am concerned that in the future it may be
> >>not soo future proof.
> >
> >Can you detail here?
> >
> >>Would it perhaps make sense to make some of the livepatching mechanism
> >>be exposed as general code? And use it for alternative asm as well?
> >
> >The code to sync the CPU looks very similar to stop_machine, so why
> >would we want to get yet another mechanism to sync the CPUs and execute
> >a specific function?
> 
> I forgot to mention that apply_alternatives_all is only called one time
> during the boot and before CPU0 goes in idle_loop. If we have to patch
> afterward, we would use apply_alternatives which consider that all the CPUs
> have been parked somewhere else.

Oh, in that case please just mention that in the commit description.

> 
> Regards,
> 
> -- 
> Julien Grall

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

Re: [Xen-devel] RFC: how to differentiate livepatched symbol and original symbol in Xen hypervisor

[Jan Beulich]

>>> On 06.06.16 at 15:32,  wrote:
> Hi,
> 
> About the livepatch TODO: "Make XENPF_get_symbol also include Live Patch
> symbols" mentioned at http://wiki.xenproject.org/wiki/XSplice, I am 
> wondering
> how the patched function would be dumped.
> 
> For instance, if function "gnttab_usage_print_all" is livepatched, it  would
> show as symbol in both Xen hypervisor and applied livepatch. How are we 
> going
> to differentiate the old and new symbols referring to the same symbol name 
> but
> different address? One address is the original and another is the on pointed 
> by
> instruction "e9 ".
> 
> Here is a sample on my test machine. The following is my own customized xen
> debug message in "xl debug-keys x". I am patching my own function 
> "my_old_func"
> in Xen hypervisor.
> 
> (XEN) name=my_global_domain, value=0x82d080409054, size=28, new=1
> (XEN) name=my_old_func, value=0x82d080409070, size=89, new=0
> (XEN) name=mg_data, value=0x82d08040a000, size=4, new=1
> 
> The following is the current result of XENPF_get_symbol on Dom0:
> 
> root@vm:/soft/img# cat /proc/xen/xensyms | grep my_old_func
> 82d0802465a4 T my_old_func
> 82d0802465a4 t .text.my_old_func
> 
> In this example, I livepatched "my_old_func" and thus we have two symbols
> referring the same name but different addresses now (82d0802465a4 and
> 82d080409070).
> 
> Are we going to use new nm symbol flag , append extra string in symbol name
> (e.g., my_old_func#livepatch) or this even does not matter?

While the output is clearly wrong, the problem isn't distinguishing
the symbols - that's simple: Everything outside of [_start,_end)
is in a livepatch. (One issue here would be multiple replacement of
the same symbol.) The main problem I see here is that
xensyms_read() doesn't even enumerate the new symbols.

Jan


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

Re: [Xen-devel] RFC: how to differentiate livepatched symbol and original symbol in Xen hypervisor

[Konrad Rzeszutek Wilk]

On Mon, Jun 06, 2016 at 06:32:16AM -0700, Dongli Zhang wrote:
> Hi,
> 
> About the livepatch TODO: "Make XENPF_get_symbol also include Live Patch
> symbols" mentioned at http://wiki.xenproject.org/wiki/XSplice, I am wondering
> how the patched function would be dumped.

Thank you for taking a look!

> 
> For instance, if function "gnttab_usage_print_all" is livepatched, it  would
> show as symbol in both Xen hypervisor and applied livepatch. How are we going
> to differentiate the old and new symbols referring to the same symbol name but
> different address? One address is the original and another is the on pointed 
> by
> instruction "e9 ".
> 
> Here is a sample on my test machine. The following is my own customized xen
> debug message in "xl debug-keys x". I am patching my own function 
> "my_old_func"
> in Xen hypervisor.
> 
> (XEN) name=my_global_domain, value=0x82d080409054, size=28, new=1
> (XEN) name=my_old_func, value=0x82d080409070, size=89, new=0
> (XEN) name=mg_data, value=0x82d08040a000, size=4, new=1
> 
> The following is the current result of XENPF_get_symbol on Dom0:
> 
> root@vm:/soft/img# cat /proc/xen/xensyms | grep my_old_func
> 82d0802465a4 T my_old_func
> 82d0802465a4 t .text.my_old_func
> 
> In this example, I livepatched "my_old_func" and thus we have two symbols
> referring the same name but different addresses now (82d0802465a4 and
> 82d080409070).

/me nods.
> 
> Are we going to use new nm symbol flag , append extra string in symbol name
> (e.g., my_old_func#livepatch) or this even does not matter?


It should not matter. What the /proc/xen/xensyms should return is the new
address.

For that to work the hypercall makes a call to xensyms_read and that needs to
be fixed to also look in the livepatch symbols. It probably needs an iterator
function to walk over each of the 'virtual_region', like this:

diff --git a/xen/include/xen/symbols.h b/xen/include/xen/symbols.h
index 20bbb28..5455a79 100644
--- a/xen/include/xen/symbols.h
+++ b/xen/include/xen/symbols.h
@@ -14,6 +14,9 @@ typedef const char *symbols_lookup_t(unsigned long addr,
  unsigned long *offset,
  char *namebuf);
 
+typedef int symbols_iterator_t(uint32_t *symnum, char *type,
+   unsigned long *address, char *name);
+
 /* Lookup an address. */
 const char *symbols_lookup(unsigned long addr,
unsigned long *symbolsize,
diff --git a/xen/include/xen/virtual_region.h b/xen/include/xen/virtual_region.h
index e5e58ed..bba0ac7 100644
--- a/xen/include/xen/virtual_region.h
+++ b/xen/include/xen/virtual_region.h
@@ -18,6 +18,10 @@ struct virtual_region
 /* If this is NULL the default lookup mechanism is used. */
 symbols_lookup_t *symbols_lookup;
 
+/* Walk over all of the symbols this region provides. */
+symbols_iterator_t *symbol_iterator;
+unsigned long nr_symbols;
+
 struct {
 const struct bug_frame *bugs; /* The pointer to array of bug frames. */
 size_t n_bugs;  /* The number of them. */


And xensyms_read would hook up to this.. somehow. And the livepatch.c would
hook its symbol iterator to this function as well.


> 
> Thank you very much!
> 
> Best,
> 
> Dongli Zhang

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

Re: [Xen-devel] Xen 4.7 crash

[Julien Grall]

(CC Ian, Stefano and Wei)

Hello Aaron,

On 06/06/16 14:58, Aaron Cornelius wrote:

On 6/2/2016 5:07 AM, Julien Grall wrote:

Hello Aaron,

On 02/06/2016 02:32, Aaron Cornelius wrote:

This is with a custom application, we use the libxl APIs to interact
with Xen.  Domains are created using the libxl_domain_create_new()
function, and domains are destroyed using the libxl_domain_destroy()
function.

The test in this case creates a domain, waits a minute, then
deletes/creates the next domain, waits a minute, and so on.  So I
wouldn't be surprised to see the VMID occasionally indicate there are 2
active domains since there could be one being created and one being
destroyed in a very short time.  However, I wouldn't expect to ever have
256 domains.


Your log has:

(XEN) grant_table.c:3288:d0v1 Grant release (0) ref:(9) flags:(2) dom:(0)
(XEN) grant_table.c:3288:d0v1 Grant release (1) ref:(11) flags:(2)
dom:(0)

Which suggest that some grants are still mapped in DOM0.



The CubieTruck only has 2GB of RAM, I allocate 512MB for dom0 which
means that only 48 of the the Mirage domains (with 32MB of RAM) would
work at the same time anyway.  Which doesn't account for the various
inter-domain resources or the RAM used by Xen itself.


All the pages who belongs to the domain could have been freed except the
one referenced by DOM0. So the footprint of this domain will be limited
at the time.

I would recommend you to check how many domain are running at this time
and if DOM0 effectively released all the resources.


If the p2m_teardown() function checked for NULL it would prevent the
crash, but I suspect Xen would be just as broken since all of my
resources have leaked away.  More broken in fact, since if the board
reboots at least the applications will restart and domains can be
recreated.

It certainly appears that some resources are leaking when domains are
deleted (possibly only on the ARM or ARM32 platforms).  We will try to
add some debug prints and see if we can discover exactly what is
going on.


The leakage could also happen from DOM0. FWIW, I have been able to cycle
2000 guests over the night on an ARM platforms.



We've done some more testing regarding this issue.  And further testing
shows that it doesn't matter if we delete the vchans before the domains
are deleted.  Those appear to be cleaned up correctly when the domain is
destroyed.

What does stop this issue from happening (using the same version of Xen
that the issue was detected on) is removing any non-standard xenstore
references before deleting the domain.  In this case our application
allocates permissions for created domains to non-standard xenstore
paths.  Making sure to remove those domain permissions before deleting
the domain prevents this issue from happening.


I am not sure to understand what you mean here. Could you give a quick 
example?




It does not appear to matter if we delete the standard domain xenstore
path (/local/domain/) since libxl handles removing this path when
the domain is destroyed.

Based on this I would guess that the xenstore is hanging onto the VMID.


Regards,

--
Julien Grall

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

Re: [Xen-devel] Issues with PCI-Passtrough (VT-d) in HVM with Xen 4.6

[Boris Ostrovsky]

On 06/06/2016 04:41 AM, Jan Beulich wrote:
>
>> - on the DomU - when I run that "test console" tool, the "lspci -xxx" 
>> output is different from before/after
>> not much, though, only one register(?)
>>
>> diff lspci.before-testconsole lspci.after-testconsole
>> 2c2
>> < 00: cf 15 00 00 02 01 00 02 00 00 00 ff 00 40 00 00
>> ---
>>> 00: cf 15 00 00 02 01 00 02 00 00 00 ff 00 00 00 00
> Okay, that's the Latency Timer field, and I think just like BARs we
> may need to permit restoring this field. However, please note that
> the reset being done behind the back of pciback really is the
> problem here: pciback assumes (for a reason, as you can certainly
> understand) that it has full control over config space of a passed
> through device.
>
> And actually the latency timer would, as a side effect of enabling
> bus mastering on the device (via the pci_set_master() call from
> command_write()) set the Latency Timer field properly, just that
> again pciback (and the rest of Dom0's PCI subsystem) thinks that
> bus mastering is already enabled on the device. So perhaps in
> permissive mode we should simply allow the latency timer field to
> be written, just like we allow writing various of the Command
> Register bits in that mode. Maintainers, what do you think?

Don't we currently allow it to be written unconditionally? It is no
different from accessing Cache Line Size.

Or are you suggesting to make it stricter?

>
> If we decide to go that route, I would then wonder whether
> Cache Line Size being unconditionally writable right now would
> also better be restricted to permissive mode.


-boris


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

Re: [Xen-devel] "xl vcpu-set" not persistent across reboot?

[Stefano Stabellini]

On Mon, 6 Jun 2016, Wei Liu wrote:
> On Mon, Jun 06, 2016 at 02:07:46PM +0100, Stefano Stabellini wrote:
> > On Mon, 6 Jun 2016, Jan Beulich wrote:
> > > >>> On 03.06.16 at 18:35,  wrote:
> > > > I got a patch ready.  But QEMU upstream refuses to start on the 
> > > > receiving end
> > > > with following error message:
> > > > 
> > > > qemu-system-i386: Unknown savevm section or instance 'cpu_common' 1
> > > > qemu-system-i386: load of migration failed: Invalid argument
> > > > 
> > > > With QEMU traditional HVM guest and PV guest, the guest works fine -- up
> > > > and running with all hot plugged cpus available.
> > > > 
> > > > So I think the relevant libxl information is transmitted but we also
> > > > need to fix QEMU upstream. But that's a separate issue.
> > 
> > For clarity, you have applied the patch below, started a VM, hotplugged
> > a vcpu, rebooted the guest, then migrated the VM, but at this point
> > there is an error?
> > 
> 
> Apply this patch, start a guest, hotplug some cpus, make them online
> inside guest, and xl migrate guest localhost.
> 
> You will see this in qemu log.
> 
> > What are the QEMU command line arguments at the receiving side? Are you
> > sure that the increased vcpu count is passed to the receiving end by
> > libxl? It looks like QEMU has been started passing the old vcpu count as
> > command line argument (-smp etc) at the receiving end.
> > 
> 
> The QEMU command line should be the same on the sending side.
> 
> The -smp 1,maxvcpus=4 (something like that).
> 
> Does that mean we need to somehow alter QEMU's command line?

Yes, that's right. The device state that we pass to QEMU is just the
state of the devices specified by the command line args.

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

Re: [Xen-devel] Xen 4.7 crash

[Aaron Cornelius]

On 6/2/2016 5:07 AM, Julien Grall wrote:

Hello Aaron,

On 02/06/2016 02:32, Aaron Cornelius wrote:

This is with a custom application, we use the libxl APIs to interact
with Xen.  Domains are created using the libxl_domain_create_new()
function, and domains are destroyed using the libxl_domain_destroy()
function.

The test in this case creates a domain, waits a minute, then
deletes/creates the next domain, waits a minute, and so on.  So I
wouldn't be surprised to see the VMID occasionally indicate there are 2
active domains since there could be one being created and one being
destroyed in a very short time.  However, I wouldn't expect to ever have
256 domains.


Your log has:

(XEN) grant_table.c:3288:d0v1 Grant release (0) ref:(9) flags:(2) dom:(0)
(XEN) grant_table.c:3288:d0v1 Grant release (1) ref:(11) flags:(2) dom:(0)

Which suggest that some grants are still mapped in DOM0.



The CubieTruck only has 2GB of RAM, I allocate 512MB for dom0 which
means that only 48 of the the Mirage domains (with 32MB of RAM) would
work at the same time anyway.  Which doesn't account for the various
inter-domain resources or the RAM used by Xen itself.


All the pages who belongs to the domain could have been freed except the
one referenced by DOM0. So the footprint of this domain will be limited
at the time.

I would recommend you to check how many domain are running at this time
and if DOM0 effectively released all the resources.


If the p2m_teardown() function checked for NULL it would prevent the
crash, but I suspect Xen would be just as broken since all of my
resources have leaked away.  More broken in fact, since if the board
reboots at least the applications will restart and domains can be
recreated.

It certainly appears that some resources are leaking when domains are
deleted (possibly only on the ARM or ARM32 platforms).  We will try to
add some debug prints and see if we can discover exactly what is going on.


The leakage could also happen from DOM0. FWIW, I have been able to cycle
2000 guests over the night on an ARM platforms.



We've done some more testing regarding this issue.  And further testing 
shows that it doesn't matter if we delete the vchans before the domains 
are deleted.  Those appear to be cleaned up correctly when the domain is 
destroyed.


What does stop this issue from happening (using the same version of Xen 
that the issue was detected on) is removing any non-standard xenstore 
references before deleting the domain.  In this case our application 
allocates permissions for created domains to non-standard xenstore 
paths.  Making sure to remove those domain permissions before deleting 
the domain prevents this issue from happening.


It does not appear to matter if we delete the standard domain xenstore 
path (/local/domain/) since libxl handles removing this path when 
the domain is destroyed.


Based on this I would guess that the xenstore is hanging onto the VMID.

- Aaron Cornelius

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