Re: [Xen-devel] [PATCH 03/16] x86/monitor: mechanical renames
On 7/11/2016 7:43 PM, Tamas K Lengyel wrote: On Sat, Jul 9, 2016 at 12:46 PM, Corneliu ZUZU wrote: On 7/9/2016 9:10 PM, Tamas K Lengyel wrote: On Fri, Jul 8, 2016 at 10:13 PM, Corneliu ZUZU wrote: Arch-specific vm-event functions in x86/vm_event.h -e.g. vm_event_init_domain()- don't have an 'arch_' prefix. Apply the same rule for monitor functions - originally the only two monitor functions that had an 'arch_' prefix were arch_monitor_domctl_event() and arch_monitor_domctl_op(), but I gave them that prefix because -they had a counterpart function in common code-, that being monitor_domctl(). This should actually be the other way around - ie adding the arch_ prefix to vm_event functions that lack it. Given that the majority of the arch-specific functions called from common-code don't have an 'arch_' prefix unless they have a common counterpart, I was guessing that was the rule. It made sense in my head since I saw in that the intention of avoiding naming conflicts (i.e you can't have monitor_domctl() both on the common-side and on the arch-side, so prepend 'arch_' to the latter). I noticed you guys also 'skip' the prefix when sending patches, so that reinforced my assumption. Having the arch_ prefix is helpful to know that the function is dealing with the arch specific structs and not common. Personally I don't see much use in 'knowing that the function is dealing with the arch structs' from the call-site and you can tell that from the implementation-site just by looking at the path of its source file. Also, the code is pretty much localized in the arch directory anyway so usually one wouldn't have to go back and forth between common and arch that often. What really bothers me though is always having to read 'arch_' when spelling a function-name and also that it makes the function name longer without much benefit. Your suggestion of adding it to pretty much all functions that make up the interface to common just adds to that headache. :-D Similarly that's why we have the hvm_ prefix for functions in hvm/monitor. 'hvm_' doesn't seem to me more special than 'monitor_', for instance, but maybe that's just me. Let this also be the rule for future 'arch_' functions additions, and with this patch remove the 'arch_' prefix from the monitor functions that don't have a counterpart in common-code (all but those 2 aforementioned). Even if there are no common counter-parts to the function, the arch_ prefix should remain, so I won't be able to ack this patch. Tamas Having said the above, are you still of the same opinion? Yes, I am. While it's not a hard rule to always apply these prefix, it does make sense to have them so I don't see benefit in removing the existing prefixes. Well, for one the benefit would be not confusing developers by creating inconsistencies: what's the rule here, i.e. why isn't a function such as alloc_domain_struct prefixed w/ 'arch_' but arch_domain_create is? The reason seems to be the latter having a common counterpart while the former not, at least that's what I see being done all over the code-base. Also, I've done this before and you seemed to agree: https://www.mail-archive.com/xen-devel%40lists.xen.org/msg57617.html (Q1). You also suggested creating arch-specific functions without the prefix: https://www.mail-archive.com/xen-devel%40lists.xen.org/msg57336.html . Why the sudden change of heart? 2ndly and obviously, removing the prefixes would make function names shorter and clearer (e.g. -read- "arch_vm_event_vcpu_unpause" and then read "vm_event_vcpu_unpause"). 3rd reason is that adding the prefix to -all- arch-specific functions called from common would mean having a lot new functions with the prefix. I'd read the prefix over and over again and at some point I'd get annoyed and say "ok, ok, it's arch_, I get it; why use this prefix so much again?". 4th reason is that the advantage of telling that the function accesses arch structures is much too little considering that idk, >50% of the codebase is arch-specific, so it doesn't provide much information, this categorization is too broad to deserve a special prefix. Whereas using the prefix only for functions that do have a common counterpart gives you the extra information that the 'operation' is only -partly- arch-specific, i.e. to see the whole picture, look @ the common-side implementation. Keep in mind that we'd also be -losing that information- if we were to apply the 'go with arch_ for all' rule.. (this could be a 5th reason) Adding arch_ prefix to the ones that don't already have one is optional, I was just pointing out that if you really feel like standardizing the naming convention, that's where I would like things to move towards to. Tamas I don't think I'd say this patch "standardizes the naming convention" but rather "fixes code that doesn't conform to the -already existing- standard naming convention". Above stated reasons explain why I'd rather -not- have this
[Xen-devel] Data Abort while in booting when using latest version on arm32 fastmodels
Hi, All I founded the previous post to solve the problem as the same as mine, the patch was applied in latest version, but I've got the data abort. previous post: https://lists.xen.org/archives/html/xen-devel/2013-09/msg00606.html and I referred to the wiki page: http://wiki.xen.org/wiki/Xen_ARM_with_Virtualization_Extensions/FastModels I build the latest version of Xen with command as below: # make distclean; XEN_TARGET_ARCH=arm32 CROSS_COMPILE=arm-linux-gnueabihf- debug=y CONFIG_EARLY_PRINTK=fastmodel ./configure # make xen XEN_TARGET_ARCH=arm32 CROSS_COMPILE=arm-linux-gnueabihf- debug=y CONFIG_EARLY_PRINTK=fastmodel -j 8 My fastmodels command as below: FVP_VE_Cortex-A15x4-A7x4 -a coretile.cluster0.*=./linux-system-semi.axf \ -a coretile.cluster1.*=./linux-system-semi.axf \ -C motherboard.smsc_91c111.enabled=1 -C motherboard.hostbridge.userNetworking=1 \ -C coretile.dualclustersystemconfigurationblock.CFG_ACTIVECLUSTER=0x3 \ -C coretile.cluster0.cpu0.semihosting-cmd_line=" \ --kernel ../xen/xen/xen \ --module ../linux/arch/arm/boot/zImage \ --dtb rtsm_ve-cortex_a15x4_a7x4.dtb \ -- earlyprintk console=ttyAMA0 mem=2048M root=/dev/nfs nfsroot=192.168.0.8:/srv/nfsroot/ rw ip=dhcp" Does anybody help me to fix it? or If I did something wrong, please let me know. here is log: Trying 127.0.0.1... Connected to localhost. Escape character is '^]'. - UART enabled - - CPU booting - - Xen starting in Hyp mode - - Zero BSS - - Setting up control registers - - Turning on paging - - Ready - (XEN) Checking for initrd in /chosen (XEN) RAM: 8000 - (XEN) (XEN) MODULE[0]: 80d0230c - 80d0473d Device Tree (XEN) MODULE[1]: 80a0 - 80d904b8 Kernel console=ttyAMA0 (XEN) (XEN) (XEN) Command line: earlyprintk console=ttyAMA0 mem=2048M root=/dev/nfs nfsroot=192.168.0.8:/srv/nfsroot/ rw ip=dhcp (XEN) Placing Xen at 0xffe0-0x0001 (XEN) Update BOOTMOD_XEN from 8020-802fd781 => ffe0-ffefd781 (XEN) Xen heap: fa00-fe00 (16384 pages) (XEN) Dom heap: 507904 pages (XEN) Domain heap initialised (XEN) Platform: VERSATILE EXPRESS (XEN) Bad console= option 'ttyAMA0' Xen 4.8-unstable (XEN) Xen version 4.8-unstable (wonseok@) (arm-linux-gnueabihf-gcc (crosstool-NG linaro-1.13.1-4.7-2013.03-20130313 - Linaro GCC 2013.03) 4.7.3 20130226 (prerelease)) debug=y Tue Jul 12 13:29:41 KST 2016 (XEN) Latest ChangeSet: Thu Jun 30 14:01:02 2016 +0200 git:bb4f41b-dirty (XEN) Processor: 412fc0f0: "ARM Limited", variant: 0x2, part 0xc0f, rev 0x0 (XEN) 32-bit Execution: (XEN) Processor Features: 1131:00011011 (XEN) Instruction Sets: AArch32 A32 Thumb Thumb-2 ThumbEE Jazelle (XEN) Extensions: GenericTimer Security (XEN) Debug Features: 02010555 (XEN) Auxiliary Features: (XEN) Memory Model Features: 10201105 2000 0124 02102211 (XEN) ISA Features: 02101110 13112111 21232041 2131 10011142 (XEN) Set SYS_FLAGS to ffe0004c (0020004c) (XEN) Generic Timer IRQ: phys=30 hyp=26 virt=27 Freq: 24000 KHz (XEN) GICv2 initialization: (XEN) gic_dist_addr=2c001000 (XEN) gic_cpu_addr=2c002000 (XEN) gic_hyp_addr=2c004000 (XEN) gic_vcpu_addr=2c006000 (XEN) gic_maintenance_irq=25 (XEN) GICv2: 128 lines, 8 cpus, secure (IID 3902043b). (XEN) Using scheduler: SMP Credit Scheduler (credit) (XEN) Allocated console ring of 64 KiB. (XEN) VFP implementer 0x41 architecture 4 part 0x30 variant 0xf rev 0x0 (XEN) Bringing up CPU1 - CPU 0001 booting - - Xen starting in Hyp mode - - Setting up control registers - - Turning on paging - - Ready - (XEN) CPU 2 booted. (XEN) Bringing up CPU3 - CPU 0003 booting - - Xen starting in Hyp mode - - Setting up control registers - - Turning on paging - - Ready - (XEN) CPU 3 booted. (XEN) Bringing up CPU4 - CPU 0100 booting - - Xen starting in Hyp mode - - Setting up control registers - - Turning on paging - - Ready - (XEN) CPU 4 booted. (XEN) Bringing up CPU5 - CPU 0101 booting - - Xen starting in Hyp mode - - Setting up control registers - - Turning on paging - - Ready - (XEN) CPU 5 booted. (XEN) Bringing up CPU6 - CPU 0102 booting - - Xen starting in Hyp mode - - Setting up control registers - - Turning on paging - - Ready - (XEN) CPU 6 booted. (XEN) Bringing up CPU7 - CPU 0103 booting - - Xen starting in Hyp mode - - Setting up control registers - - Turning on paging - - Ready - (XEN) CPU 7 booted. (XEN) Brought up 8 CPUs (XEN) P2M: 40-bit IPA (XEN) P2M: 3 levels with order-1 root, VTCR 0x80003558 (XEN) I/O virtualisation disabled (
Re: [Xen-devel] [PATCH 02/16] x86: fix: make atomic_read() param const
Hi Andrew,
On 7/11/2016 6:18 PM, Andrew Cooper wrote:
On 09/07/16 05:12, Corneliu ZUZU wrote:
This wouldn't let me make a param of a function that used atomic_read() const.
Signed-off-by: Corneliu ZUZU
This is a good improvement, but you must make an identical adjustment to
the arm code, otherwise you will end up with subtle build failures.
Right, didn't even realize it was X86-specific.
If you are really feeling up to it, having a common xen/atomic.h with
typedef struct { int counter; } atomic_t;
#define ATOMIC_INIT(i) { (i) }
and some prototypes such as:
static inline int atomic_read(const atomic_t *v);
would be great, but this looks like it has the possibility to turn into
a rats nest. If it does, then just doubling up this code for arm is ok.
~Andrew
Yes, that might be more complicated than we expect and I don't know if
making code such as this common would be a good idea, usually these
functions are always architecture-specific. It might be better to keep
them separate - they don't add much anyway since their implementation is
short - than risk unexpected different behavior on a future arch. But
then again I don't know much details of their implementation, so anyway,
I'd surely prefer to do this kind of change in a separate patch.
---
xen/include/asm-x86/atomic.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/xen/include/asm-x86/atomic.h b/xen/include/asm-x86/atomic.h
index d246b70..0b250c8 100644
--- a/xen/include/asm-x86/atomic.h
+++ b/xen/include/asm-x86/atomic.h
@@ -94,7 +94,7 @@ typedef struct { int counter; } atomic_t;
*
* Atomically reads the value of @v.
*/
-static inline int atomic_read(atomic_t *v)
+static inline int atomic_read(const atomic_t *v)
{
return read_atomic(&v->counter);
}
@@ -105,7 +105,7 @@ static inline int atomic_read(atomic_t *v)
*
* Non-atomically reads the value of @v
*/
-static inline int _atomic_read(atomic_t v)
+static inline int _atomic_read(const atomic_t v)
{
return v.counter;
}
Thanks,
Zuzu C.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] linux-next: manual merge of the xen-tip tree with the pm tree
Hi all,
Today's linux-next merge of the xen-tip tree got a conflict in:
drivers/acpi/scan.c
between commit:
68bdb6773289 ("ACPI: add support for ACPI reconfiguration notifiers")
from the pm tree and commit:
c8ac8b6fb495 ("Xen: ACPI: Hide UART used by Xen")
from the xen-tip tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc drivers/acpi/scan.c
index 405056b95b05,cfc73fecaba4..
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@@ -1925,8 -1961,19 +1970,21 @@@ static int acpi_bus_scan_fixed(void
return result < 0 ? result : 0;
}
+ static void __init acpi_get_spcr_uart_addr(void)
+ {
+ acpi_status status;
+ struct acpi_table_spcr *spcr_ptr;
+
+ status = acpi_get_table(ACPI_SIG_SPCR, 0,
+ (struct acpi_table_header **)&spcr_ptr);
+ if (ACPI_SUCCESS(status))
+ spcr_uart_addr = spcr_ptr->serial_port.address;
+ else
+ printk(KERN_WARNING PREFIX "STAO table present, but SPCR is
missing\n");
+ }
+
+static bool acpi_scan_initialized;
+
int __init acpi_scan_init(void)
{
int result;
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] Atheros WiFi - memory paging failure on driver load
Hello Some background - We are trying to run Qualcomm Atheros AR928X Wireless Network Adapter and have a crash right on driver load, following are our observations and questions. Jurgen's observation - " The Atheros card "Qualcomm Atheros AR928X Wireless Network Adapter (PCI-Express) (rev 01)" is plugged into the host system (datatron). When I attach it to the DomU - the module "ath9k" is automatically loaded, but it gives an exception "iowrite32+0x2b/0x30". No idea what the issue is (tried also with another Atheros Card (ath10k) - similar problem). When I try an Intel card, it works. (the card also works on the Dom0 - so the Linux driver and HW is OK)." Debugging - After some investigation with kgdb and iommu trace on DomU it seems the iomap of PCI BAR for the device returns a a mapping f which first 0x1000 bytes are read only and that causes access violation when trying to write registers mapped to this area (all the regs with offset < 0x1000) - why this happens i still don't know. Register writes with offsets > 0x1000 are fine. Running same driver on Dom0 is totally fine Bellow the sigsev backtrace and xen dmesg from DomU As can be seen there is ioremap_ of size 0x1 starting at c900402c but as i said, i noticed that anything bellow c900402c*1000 *is not writable (from gdb using set addr = val) and only readable while anything above this address is writeable. Question - Please give any advise on this issue and especially how to approach debugging this both on Domu and Dom0 and where in xen code to look for possible issues. Thanks. Andrey P.S I was also unable to cat /sys/kernel/debug/tracing/trace_pipe > mydump.txt & to actually record the iommu traces due to another paging crash (stack in the end) [ 37.837467] ath9k :00:00.0: Xen PCI mapped GSI16 to IRQ8 [ 37.837473] pcifront pci-0: read dev=:00:00.0 - offset 3d size 1 [ 37.837498] pcifront pci-0: read got back value 1 [ 37.837505] pcifront pci-0: read dev=:00:00.0 - offset 4 size 2 [ 37.840006] pcifront pci-0: read got back value 103 [ 37.853341] pcifront pci-0: read dev=:00:00.0 - offset c size 1 [ 37.853374] pcifront pci-0: read got back value 8 [ 37.853383] pcifront pci-0: write dev=:00:00.0 - offset d size 1 val a8 [ 37.853431] pcifront pci-0: read dev=:00:00.0 - offset 4 size 2 [ 37.853462] pcifront pci-0: read got back value 103 [ 37.853472] pcifront pci-0: write dev=:00:00.0 - offset 4 size 2 val 107 [ 37.853527] pcifront pci-0: read dev=:00:00.0 - offset 40 size 4 [ 37.853565] pcifront pci-0: read got back value 3c25001 [ 37.853574] pcifront pci-0: write dev=:00:00.0 - offset 40 size 4 val 3c20001 *[ 37.855784] mmiotrace: ioremap_*(0xf7b0, 0x1) = c900402c* [ 37.855842] pcifront pci-0: read dev=:00:00.0 - offset c size 1 [ 37.855879] pcifront pci-0: read got back value 8 [ 38.301919] BUG: unable to handle kernel paging request at c900402c0040 [ 38.301930] IP: [] iowrite32+0x2b/0x30 [ 38.301939] PGD 3fdf4067 PUD 3e1a8067 PMD 49d7067 PTE 8010f7b00075 [ 38.301947] Oops: 0003 [#1] SMP [ 38.301952] Modules linked in: ath9k(OE+) ath9k_common(OE) ath9k_hw(OE) ath(OE) mac80211(E) cfg80211(E) rfkill(E) xen_pcifront(E) intel_rapl(E) x86_pkg_temp_thermal(E) coretemp(E) crct10dif_pclmul(E) crc32_pclmul(E) evdev(E) ghash_clmulni_intel(E) pcspkr(E) uio_netx(E) uio(E) autofs4(E) ext4(E) ecb(E) crc16(E) mbcache(E) jbd2(E) crc32c_generic(E) crc32c_intel(E) aesni_intel(E) xen_netfront(E) xen_blkfront(E) aes_x86_64(E) glue_helper(E) lrw(E) gf128mul(E) ablk_helper(E) cryptd(E) [ 38.302000] CPU: 0 PID: 696 Comm: systemd-udevd Tainted: GW OE 4.5.3-dbg #2 [ 38.302008] task: 8800044880c0 ti: 8800048a8000 task.ti: 8800048a8000 [ 38.302015] RIP: e030:[] [] iowrite32+0x2b/0x30 [ 38.302024] RSP: e02b:8800048ab8c8 EFLAGS: 00010196 [ 38.302029] RAX: RBX: 000e RCX: 88003ac34018 [ 38.302035] RDX: c900402c0040 RSI: c900402c0040 RDI: [ 38.302040] RBP: 8800048ab928 R08: 0016 R09: 0002 [ 38.302046] R10: 81b08000 R11: 81b07fc0 R12: 0016 [ 38.302051] R13: 880004b4d098 R14: 8800048abec0 R15: c03c74c0 [ 38.302060] FS: 7ff256fb18c0() GS:88003f80() knlGS:88003f80 [ 38.302068] CS: e033 DS: ES: CR0: 80050033 [ 38.302073] CR2: c900402c0040 CR3: 04a05000 CR4: 00042660 [ 38.302079] Stack: [ 38.302083] c03a70f5 0040 88003ac34018 c0339a5b [ 38.302092] 88003ac34018 88003ac34068 88003ac81520 80948090 [ 38.302102] 8098 3c5869cf [ 38.302111] Call Trace: [ 38.302123] [] ? ath9k_iowrite32+0xde/0xf5 [ath9k] [ 38.302139] [] ? ath9k_hw_update_mibstats+0x62/0xd6 [ath9k_hw] [ 38.302155] [] ? ath
Re: [Xen-devel] [PATCH v2 16/17] libxc/xc_dom_arm: Copy ACPI tables to guest space
On 2016/7/6 18:12, Stefano Stabellini wrote:
> On Wed, 6 Jul 2016, Julien Grall wrote:
>> > Hi Stefano,
>> >
>> > On 05/07/16 18:13, Stefano Stabellini wrote:
>>> > > On Thu, 23 Jun 2016, Julien Grall wrote:
> > > On 23/06/2016 04:17, Shannon Zhao wrote:
> > > > > From: Shannon Zhao
> > > > >
> > > > > Copy all the ACPI tables to guest space so that UEFI or guest
> > > > > could
> > > > > access them.
> > > > >
> > > > > Signed-off-by: Shannon Zhao
> > > > > ---
> > > > > tools/libxc/xc_dom_arm.c | 51
> > > > >
> > > > > 1 file changed, 51 insertions(+)
> > > > >
> > > > > diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
> > > > > index 64a8b67..6a0a5b7 100644
> > > > > --- a/tools/libxc/xc_dom_arm.c
> > > > > +++ b/tools/libxc/xc_dom_arm.c
> > > > > @@ -63,6 +63,47 @@ static int setup_pgtables_arm(struct
> > > > > xc_dom_image
> > > > > *dom)
> > > > >
> > > > > /*
> > > > >
> > > > > */
> > > > >
> > > > > +static int xc_dom_copy_acpi(struct xc_dom_image *dom)
> > > > > +{
> > > > > +int rc, i;
> > > > > +uint32_t pages_num = ROUNDUP(dom->acpitable_size,
> > > > > XC_PAGE_SHIFT) >>
> > > > > + XC_PAGE_SHIFT;
> > > > > +const xen_pfn_t base = GUEST_ACPI_BASE >> XC_PAGE_SHIFT;
> > > > > +xen_pfn_t *p2m;
> > > > > +void *acpi_pages;
> > > > > +
> > > > > +p2m = malloc(pages_num * sizeof(*p2m));
> > > > > +for (i = 0; i < pages_num; i++)
> > > > > +p2m[i] = base + i;
> > > > > +
> > > > > +rc = xc_domain_populate_physmap_exact(dom->xch,
> > > > > dom->guest_domid,
> > > > > + pages_num, 0, 0, p2m);
> > >
> > > H... it looks like this is working because libxl is setting the
> > > maximum
> > > size of the domain with some slack (1MB). However, I guess the slack
> > > was
> > > for
> > > something else. Wei, Stefano, Ian, can you confirm?
>>> > >
>>> > > If I recall correctly, the slack is a magic value coming from the
>>> > > ancient history of toolstacks.
>> >
>> > Does it mean we would need to update the slack to take into account the
>> > ACPI
>> > blob?
> Yes, we need to take into account the ACPI blob. Probably not in the
> slack but directly in mam_memkb.
Sorry, I'm not sure understand this. I found the b_info->max_memkb but
didn't find the slack you said. And how to fix this? Update
b_info->max_memkb or the slack?
Thanks,
--
Shannon
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 05/17] libxl/arm: Generate static ACPI DSDT table
On 2016/7/7 23:52, Wei Liu wrote: > On Tue, Jul 05, 2016 at 11:12:35AM +0800, Shannon Zhao wrote: >> > From: Shannon Zhao >> > >> > It uses static DSDT table like the way x86 uses. Currently the DSDT >> > table only contains processor device objects and it generates the >> > maximal objects which so far is 128. >> > >> > Signed-off-by: Shannon Zhao >> > --- >> > tools/libacpi/Makefile| 15 - >> > tools/libacpi/mk_dsdt.c | 51 >> > --- >> > tools/libxl/Makefile | 5 - >> > tools/libxl/libxl_arm_acpi.c | 5 + >> > xen/include/public/arch-arm.h | 3 +++ >> > 5 files changed, 64 insertions(+), 15 deletions(-) >> > >> > diff --git a/tools/libacpi/Makefile b/tools/libacpi/Makefile >> > index 4068d9a..0401810 100644 >> > --- a/tools/libacpi/Makefile >> > +++ b/tools/libacpi/Makefile >> > @@ -22,6 +22,7 @@ MK_DSDT = $(ACPI_BUILD_DIR)/mk_dsdt >> > # Sources to be generated >> > C_SRC = $(ACPI_BUILD_DIR)/dsdt_anycpu.c $(ACPI_BUILD_DIR)/dsdt_15cpu.c >> > C_SRC += $(ACPI_BUILD_DIR)/dsdt_anycpu_qemu_xen.c >> > $(ACPI_BUILD_DIR)/dsdt_pvh.c >> > +C_SRC += $(ACPI_BUILD_DIR)/dsdt_anycpu_arm.c >> > H_SRC = $(ACPI_BUILD_DIR)/ssdt_s3.h $(ACPI_BUILD_DIR)/ssdt_s4.h >> > $(ACPI_BUILD_DIR)/ssdt_pm.h $(ACPI_BUILD_DIR)/ssdt_tpm.h >> > >> > vpath iasl $(PATH) >> > @@ -35,7 +36,7 @@ $(H_SRC): $(ACPI_BUILD_DIR)/%.h: %.asl iasl >> >cd $(CURDIR) >> > >> > $(MK_DSDT): mk_dsdt.c >> > - $(HOSTCC) $(HOSTCFLAGS) $(CFLAGS_xeninclude) -o $@ mk_dsdt.c >> > + $(HOSTCC) $(HOSTCFLAGS) $(CFLAGS_xeninclude) -D__XEN_TOOLS__ -o $@ >> > mk_dsdt.c > Why is this needed? Which unstable hypervisor interface you need in > order to build this? It needs GUEST_MAX_VCPUS in mk_dsdt.c while the GUEST_MAX_VCPUS is defined under #if defined(__XEN__) || defined(__XEN_TOOLS__) in xen/include/public/arch-arm.h Thanks, -- Shannon ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 03/17] libxl/arm: Add a configuration option for ARM DomU ACPI
On 2016/7/7 23:30, Wei Liu wrote:
> On Mon, Jun 27, 2016 at 11:40:32AM +0100, Julien Grall wrote:
>> > Hi Shannon,
>> >
>> > On 23/06/16 15:34, Shannon Zhao wrote:
>>> > >On 2016年06月23日 21:39, Stefano Stabellini wrote:
> >>On Thu, 23 Jun 2016, Shannon Zhao wrote:
>> > From: Shannon Zhao
>> >
>> > Add a configuration option for ARM DomU so that user can deicde to
>> > use
>> > ACPI or not. This option is defaultly false.
>> >
>> > Signed-off-by: Shannon Zhao
>> > ---
>> > tools/libxl/libxl_arm.c | 3 +++
>> > tools/libxl/libxl_types.idl | 1 +
>> > tools/libxl/xl_cmdimpl.c | 4
>> > xen/include/public/arch-arm.h | 1 +
>> > 4 files changed, 9 insertions(+)
>> >
>> > diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
>> > index 8f15d9b..cc5a717 100644
>> > --- a/tools/libxl/libxl_arm.c
>> > +++ b/tools/libxl/libxl_arm.c
>> > @@ -77,6 +77,9 @@ int libxl__arch_domain_prepare_config(libxl__gc
>> > *gc,
>> > return ERROR_FAIL;
>> > }
>> >
>> > +xc_config->acpi =
>> > libxl_defbool_val(d_config->b_info.arch_arm.acpi)
>> > + ? true : false;
>> > +
>> > return 0;
>> > }
>> >
>> > diff --git a/tools/libxl/libxl_types.idl
>> > b/tools/libxl/libxl_types.idl
>> > index ef614be..426b868 100644
>> > --- a/tools/libxl/libxl_types.idl
>> > +++ b/tools/libxl/libxl_types.idl
>> > @@ -560,6 +560,7 @@ libxl_domain_build_info =
>> > Struct("domain_build_info",[
>> >
>> >
>> > ("arch_arm", Struct(None, [("gic_version",
>> > libxl_gic_version),
>> > + ("acpi", libxl_defbool),
>> > ])),
>> >
>> > ], dir=DIR_IN
>> > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
>> > index 6459eec..0634ffa 100644
>> > --- a/tools/libxl/xl_cmdimpl.c
>> > +++ b/tools/libxl/xl_cmdimpl.c
>> > @@ -2506,6 +2506,10 @@ skip_usbdev:
>> > }
>> > }
>> >
>> > +if (xlu_cfg_get_defbool(config, "acpi",
>> > &b_info->arch_arm.acpi, 0)) {
>> > +libxl_defbool_set(&b_info->arch_arm.acpi, 0);
>> > +}
> >>We cannot share the existing code to parse the acpi paramter because
> >>that is saved in b_info->u.hvm.acpi, right?
>>> > >Yes.
> >>It's a pity. I wonder if we
> >>could refactor the existing code so that we can actually share the acpi
> >>parameter between x86 and arm.
> >>
>>> > >I have no idea about this since I'm not familiar with this. But is there
>>> > >any downsides of current way? Because for x86, it will use
>>> > >b_info->u.hvm.acpi and for ARM it will use b_info->arch_arm.acpi. I
>>> > >think they don't conflict even though we store it at two places.
>> >
>> > Yes, there is a downside. Toolstack, such as libvirt, would need to have
>> > separate code for x86 and ARM in order to enable/disable ACPI.
>> >
>> > I would introduce a new generic acpi parameters, deprecate
>> > b_info->u.hvm.acpi. Ian, Stefano, Wei, any opinions?
>> >
> Yeah, we can deprecate that field. But we need to take care to not break
> users of the old field.
Ok, what name would you suggest?
Thanks,
--
Shannon
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [oxenstored]Guest users could get the VM count and domids on the host
Hi all, I found a problem in oxenstored, which may be a security issue: Guest users could get the VM count and domids on the host by a sniffing method. You can reproduce it like this: (1) Create a VM, e.g. CentOS 7.0 64bit (2) Install xen tools in VM, excute cmds: yum install centos-release-xen; yum install (3) Use xenstore-ls to sniff, excute cmds: for((i=1;i<=1000;i++));do `xenstore-ls /local/domain/$i 1>>1.txt 2>>2.txt`; done then check 2.txt, speculate according the error message. example: xenstore-ls: xs_directory (/local/domain/17): No such file or directory ---which means dom 17 does not exist xenstore-ls: xs_directory (/local/domain/19): Permission denied ---which means dom 19 exists Count the number of "Permission denied" and we get the VM count on the host. I tried xen-4.2 and xen-4.6, same result with above. But when I use c-xenstored on xen-4.2, all error messages are "Permission denied", so there is no way to get any info about other domains on the host. In func "get_node" of c-xenstored, it will clean up the errno before return: /* Clean up errno if they weren't supposed to know. */ if (!node) errno = errno_from_parents(conn, name, errno, perm); return node; but in oxenstored, there is no such code like this. So, I think this part was missed when we upgraded c-xenstored to oxenstored. Please confirm. Looking forward to your reply, thank you! Regards, Jason ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2] xen_pvscsi: reclaim the ring request when the prepairing failed
During scsi command queueing or exception handling, if prepairing fails, we need to reclaim the failed request. Otherwise, the garbage request will be pushed into the ring for the backend to work. Signed-off-by: Bin Wu --- drivers/scsi/xen-scsifront.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/xen-scsifront.c b/drivers/scsi/xen-scsifront.c index 9dc8687..8646db1 100644 --- a/drivers/scsi/xen-scsifront.c +++ b/drivers/scsi/xen-scsifront.c @@ -184,8 +184,6 @@ static struct vscsiif_request *scsifront_pre_req(struct vscsifrnt_info *info) ring_req = RING_GET_REQUEST(&(info->ring), ring->req_prod_pvt); - ring->req_prod_pvt++; - ring_req->rqid = (uint16_t)id; return ring_req; @@ -196,6 +194,8 @@ static void scsifront_do_request(struct vscsifrnt_info *info) struct vscsiif_front_ring *ring = &(info->ring); int notify; + ring->req_prod_pvt++; + RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(ring, notify); if (notify) notify_remote_via_irq(info->irq); -- 2.3.2 (Apple Git-55) ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen_pvscsi: reclaim the ring request when mapping data failed
On 2016/7/11 17:53, Juergen Gross wrote:
On 11/07/16 11:50, David Vrabel wrote:
On 11/07/16 10:33, Juergen Gross wrote:
On 11/07/16 04:51, Bin Wu wrote:
During scsi command queueing, if mapping data fails, we need to
reclaim the failed request. Otherwise, the garbage request will
be pushed into the ring for the backend to work.
Well spotted. There is another instance of this problem in
scsifront_action_handler(). Would you mind correcting this one, too?
Would it make more sense to advance req_prod_pvt only if the request has
been successfully created?
Yeah, probably as the first action in scsifront_do_request().
Juergen
ok, I will send a new patch : )
David
Signed-off-by: Bin Wu
---
drivers/scsi/xen-scsifront.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/scsi/xen-scsifront.c b/drivers/scsi/xen-scsifront.c
index 9dc8687..655163d 100644
--- a/drivers/scsi/xen-scsifront.c
+++ b/drivers/scsi/xen-scsifront.c
@@ -565,6 +565,7 @@ static int scsifront_queuecommand(struct Scsi_Host
*shost,
err = map_data_for_request(info, sc, ring_req, shadow);
if (err < 0) {
pr_debug("%s: err %d\n", __func__, err);
+info->ring.req_prod_pvt--;
scsifront_put_rqid(info, rqid);
scsifront_return(info);
spin_unlock_irqrestore(shost->host_lock, flags);
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 11/16] x86/monitor: fix: treat -monitor- properly, as a subsys of the vm-event subsys
On 7/12/2016 12:27 AM, Tamas K Lengyel wrote:
On Mon, Jul 11, 2016 at 2:20 PM, Corneliu ZUZU wrote:
On 7/11/2016 7:38 PM, Tamas K Lengyel wrote:
diff --git a/xen/include/asm-arm/monitor.h
b/xen/include/asm-arm/monitor.h
index 9a9734a..7ef30f1 100644
[snip]
I keep seeing '[snip]' lately but I don't know what it means.
Placeholder for "I've cut some of your patch content here to shorten
the message I'm sending".
diff --git a/xen/include/xen/monitor.h b/xen/include/xen/monitor.h
index 2171d04..605caf0 100644
--- a/xen/include/xen/monitor.h
+++ b/xen/include/xen/monitor.h
@@ -22,12 +22,15 @@
#ifndef __XEN_MONITOR_H__
#define __XEN_MONITOR_H__
-#include
-
-struct domain;
-struct xen_domctl_monitor_op;
+#include
int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op
*op);
+
+static inline bool_t monitor_domain_initialised(const struct domain
*d)
+{
+return d->monitor.initialised;
This should be !!d->monitor.initialised.
It's the value of a bit, thus should be 0 or 1. Am I missing something?
Using a bitfield is effectively doing a bitmask for the bit(s).
However, returning the value after applying a bitmask is not
necessarily 0/1 (ie. bool_t). For example I ran into problems with
this in the past (see
https://lists.xen.org/archives/html/xen-devel/2015-08/msg01948.html).
Tamas
The example you provided actually returns a value &-ed with a flag
(bitmask), of course it returns either 0 or the flag itself :-). AFAIK a
single-bit item in a bitfield (note a -bitfield-, not e.g. an unsigned int
&-ed with (1<
I would assume as well but I'm not actually sure if that's guaranteed
or if it's only compiler defined behavior.
Tamas
As per http://en.cppreference.com/w/c/language/bit_field .
"The number of bits in a bit field (width) sets the limit to the range
of values it can hold"
So if it's width is 1, it can either be 0 or 1.
Corneliu.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 11/16] x86/monitor: fix: treat -monitor- properly, as a subsys of the vm-event subsys
On Mon, Jul 11, 2016 at 2:20 PM, Corneliu ZUZU wrote:
> On 7/11/2016 7:38 PM, Tamas K Lengyel wrote:
>
> diff --git a/xen/include/asm-arm/monitor.h
> b/xen/include/asm-arm/monitor.h
> index 9a9734a..7ef30f1 100644
[snip]
>>>
>>>
>>> I keep seeing '[snip]' lately but I don't know what it means.
>>
>> Placeholder for "I've cut some of your patch content here to shorten
>> the message I'm sending".
>>
> diff --git a/xen/include/xen/monitor.h b/xen/include/xen/monitor.h
> index 2171d04..605caf0 100644
> --- a/xen/include/xen/monitor.h
> +++ b/xen/include/xen/monitor.h
> @@ -22,12 +22,15 @@
>#ifndef __XEN_MONITOR_H__
>#define __XEN_MONITOR_H__
>
> -#include
> -
> -struct domain;
> -struct xen_domctl_monitor_op;
> +#include
>
>int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op
> *op);
> +
> +static inline bool_t monitor_domain_initialised(const struct domain
> *d)
> +{
> +return d->monitor.initialised;
This should be !!d->monitor.initialised.
>>>
>>>
>>> It's the value of a bit, thus should be 0 or 1. Am I missing something?
>>
>> Using a bitfield is effectively doing a bitmask for the bit(s).
>> However, returning the value after applying a bitmask is not
>> necessarily 0/1 (ie. bool_t). For example I ran into problems with
>> this in the past (see
>> https://lists.xen.org/archives/html/xen-devel/2015-08/msg01948.html).
>>
>> Tamas
>
>
> The example you provided actually returns a value &-ed with a flag
> (bitmask), of course it returns either 0 or the flag itself :-). AFAIK a
> single-bit item in a bitfield (note a -bitfield-, not e.g. an unsigned int
> &-ed with (1<
I would assume as well but I'm not actually sure if that's guaranteed
or if it's only compiler defined behavior.
Tamas
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 11/16] x86/monitor: fix: treat -monitor- properly, as a subsys of the vm-event subsys
On 7/11/2016 7:38 PM, Tamas K Lengyel wrote:
diff --git a/xen/include/asm-arm/monitor.h
b/xen/include/asm-arm/monitor.h
index 9a9734a..7ef30f1 100644
[snip]
I keep seeing '[snip]' lately but I don't know what it means.
Placeholder for "I've cut some of your patch content here to shorten
the message I'm sending".
diff --git a/xen/include/xen/monitor.h b/xen/include/xen/monitor.h
index 2171d04..605caf0 100644
--- a/xen/include/xen/monitor.h
+++ b/xen/include/xen/monitor.h
@@ -22,12 +22,15 @@
#ifndef __XEN_MONITOR_H__
#define __XEN_MONITOR_H__
-#include
-
-struct domain;
-struct xen_domctl_monitor_op;
+#include
int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *op);
+
+static inline bool_t monitor_domain_initialised(const struct domain *d)
+{
+return d->monitor.initialised;
This should be !!d->monitor.initialised.
It's the value of a bit, thus should be 0 or 1. Am I missing something?
Using a bitfield is effectively doing a bitmask for the bit(s).
However, returning the value after applying a bitmask is not
necessarily 0/1 (ie. bool_t). For example I ran into problems with
this in the past (see
https://lists.xen.org/archives/html/xen-devel/2015-08/msg01948.html).
Tamas
The example you provided actually returns a value &-ed with a flag
(bitmask), of course it returns either 0 or the flag itself :-). AFAIK a
single-bit item in a bitfield (note a -bitfield-, not e.g. an unsigned
int &-ed with (1<
Corneliu.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v6] x86/mem-sharing: mem-sharing a range of memory
Currently mem-sharing can be performed on a page-by-page basis from the control
domain. However, this process is quite wasteful when a range of pages have to
be deduplicated.
This patch introduces a new mem_sharing memop for range sharing where
the user doesn't have to separately nominate each page in both the source and
destination domain, and the looping over all pages happen in the hypervisor.
This significantly reduces the overhead of sharing a range of memory.
Signed-off-by: Tamas K Lengyel
---
Cc: Ian Jackson
Cc: Wei Liu
Cc: George Dunlap
Cc: Jan Beulich
Cc: Andrew Cooper
---
tools/libxc/include/xenctrl.h| 15
tools/libxc/xc_memshr.c | 19 +
tools/tests/mem-sharing/memshrtool.c | 22 ++
xen/arch/x86/mm/mem_sharing.c| 140 +++
xen/include/public/memory.h | 10 ++-
5 files changed, 205 insertions(+), 1 deletion(-)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 4a85b4a..650a763 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2333,6 +2333,21 @@ int xc_memshr_add_to_physmap(xc_interface *xch,
domid_t client_domain,
unsigned long client_gfn);
+/* Allows to deduplicate a range of memory of a client domain. Using
+ * this function is equivalent of calling xc_memshr_nominate_gfn for each gfn
+ * in the two domains followed by xc_memshr_share_gfns.
+ *
+ * May fail with -EINVAL if the source and client domain have different
+ * memory size or if memory sharing is not enabled on either of the domains.
+ * May also fail with -ENOMEM if there isn't enough memory available to store
+ * the sharing metadata before deduplication can happen.
+ */
+int xc_memshr_range_share(xc_interface *xch,
+ domid_t source_domain,
+ domid_t client_domain,
+ unsigned long start,
+ unsigned long end);
+
/* Debug calls: return the number of pages referencing the shared frame backing
* the input argument. Should be one or greater.
*
diff --git a/tools/libxc/xc_memshr.c b/tools/libxc/xc_memshr.c
index deb0aa4..6b26f3f 100644
--- a/tools/libxc/xc_memshr.c
+++ b/tools/libxc/xc_memshr.c
@@ -181,6 +181,25 @@ int xc_memshr_add_to_physmap(xc_interface *xch,
return xc_memshr_memop(xch, source_domain, &mso);
}
+int xc_memshr_range_share(xc_interface *xch,
+ domid_t source_domain,
+ domid_t client_domain,
+ unsigned long start,
+ unsigned long end)
+{
+xen_mem_sharing_op_t mso;
+
+memset(&mso, 0, sizeof(mso));
+
+mso.op = XENMEM_sharing_op_range_share;
+
+mso.u.range.client_domain = client_domain;
+mso.u.range.start = start;
+mso.u.range.end = end;
+
+return xc_memshr_memop(xch, source_domain, &mso);
+}
+
int xc_memshr_domain_resume(xc_interface *xch,
domid_t domid)
{
diff --git a/tools/tests/mem-sharing/memshrtool.c
b/tools/tests/mem-sharing/memshrtool.c
index 437c7c9..bd1f4e0 100644
--- a/tools/tests/mem-sharing/memshrtool.c
+++ b/tools/tests/mem-sharing/memshrtool.c
@@ -24,6 +24,8 @@ static int usage(const char* prog)
printf(" nominate- Nominate a page for sharing.\n");
printf(" share
\n");
printf(" - Share two pages.\n");
+printf(" range
\n");
+printf(" - Share pages between domains in
range.\n");
printf(" unshare - Unshare a page by grabbing a writable
map.\n");
printf(" add-to-physmap
\n");
printf(" - Populate a page in a domain with a
shared page.\n");
@@ -180,6 +182,26 @@ int main(int argc, const char** argv)
}
printf("Audit returned %d errors.\n", rc);
}
+else if( !strcasecmp(cmd, "range") )
+{
+domid_t sdomid, cdomid;
+int rc;
+unsigned long start, end;
+
+if( argc != 6 )
+return usage(argv[0]);
+sdomid = strtol(argv[2], NULL, 0);
+cdomid = strtol(argv[3], NULL, 0);
+start = strtoul(argv[4], NULL, 0);
+end = strtoul(argv[5], NULL, 0);
+
+rc = xc_memshr_range_share(xch, sdomid, cdomid, start, end);
+if ( rc < 0 )
+{
+printf("error executing xc_memshr_bulk_dedup: %s\n",
strerror(errno));
+return rc;
+}
+}
return 0;
}
diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index a522423..6d00228 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -1294,6 +1294,58 @@ int relinquish_shared_pages(struct domain *d)
return rc;
}
+static int range_share(struct domain *d, struct domain *cd,
+ struct mem_sharing_op_range *range)
+{
+int rc = 0;
+shr_handle_t sh, ch;
+unsigned l
Re: [Xen-devel] [PATCH] xen/arm: io: Protect the handlers with a read-write lock
Hi Stefano,
On 11/07/16 18:49, Stefano Stabellini wrote:
On Tue, 28 Jun 2016, Julien Grall wrote:
Currently, accessing the I/O handlers does not require to take a lock
because new handlers are always added at the end of the array. In a
follow-up patch, this array will be sort to optimize the look up.
Given that most of the time the I/O handlers will not be modify,
using a spinlock will add contention when multiple vCPU are accessing
the emulated MMIOs. So use a read-write lock to protected the handlers.
Finally, take the opportunity to re-indent correctly domain_io_init.
Signed-off-by: Julien Grall
I would appreciate if you could avoid mixing indentation changes with
other changes in the future.
The indentation changes was very small and I did not feel it was
necessary to have a separate patch for it. I tend to limit the number of
patches unless it hides important changes.
Anyway, I will try to split coding style changes and functional changes
in the future.
Reviewed-by: Stefano Stabellini
I'll commit.
Thank you!
Regards,
xen/arch/arm/io.c | 47 +++---
xen/include/asm-arm/mmio.h | 3 ++-
2 files changed, 30 insertions(+), 20 deletions(-)
diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c
index 0156755..5a96836 100644
--- a/xen/arch/arm/io.c
+++ b/xen/arch/arm/io.c
@@ -70,23 +70,39 @@ static int handle_write(const struct mmio_handler *handler,
struct vcpu *v,
handler->priv);
}
-int handle_mmio(mmio_info_t *info)
+static const struct mmio_handler *find_mmio_handler(struct domain *d,
+paddr_t gpa)
{
-struct vcpu *v = current;
-int i;
-const struct mmio_handler *handler = NULL;
-const struct vmmio *vmmio = &v->domain->arch.vmmio;
+const struct mmio_handler *handler;
+unsigned int i;
+struct vmmio *vmmio = &d->arch.vmmio;
+
+read_lock(&vmmio->lock);
for ( i = 0; i < vmmio->num_entries; i++ )
{
handler = &vmmio->handlers[i];
-if ( (info->gpa >= handler->addr) &&
- (info->gpa < (handler->addr + handler->size)) )
+if ( (gpa >= handler->addr) &&
+ (gpa < (handler->addr + handler->size)) )
break;
}
if ( i == vmmio->num_entries )
+handler = NULL;
+
+read_unlock(&vmmio->lock);
+
+return handler;
+}
+
+int handle_mmio(mmio_info_t *info)
+{
+struct vcpu *v = current;
+const struct mmio_handler *handler = NULL;
+
+handler = find_mmio_handler(v->domain, info->gpa);
+if ( !handler )
return 0;
if ( info->dabt.write )
@@ -104,7 +120,7 @@ void register_mmio_handler(struct domain *d,
BUG_ON(vmmio->num_entries >= MAX_IO_HANDLER);
-spin_lock(&vmmio->lock);
+write_lock(&vmmio->lock);
handler = &vmmio->handlers[vmmio->num_entries];
@@ -113,24 +129,17 @@ void register_mmio_handler(struct domain *d,
handler->size = size;
handler->priv = priv;
-/*
- * handle_mmio is not using the lock to avoid contention.
- * Make sure the other processors see the new handler before
- * updating the number of entries
- */
-dsb(ish);
-
vmmio->num_entries++;
-spin_unlock(&vmmio->lock);
+write_unlock(&vmmio->lock);
}
int domain_io_init(struct domain *d)
{
- spin_lock_init(&d->arch.vmmio.lock);
- d->arch.vmmio.num_entries = 0;
+rwlock_init(&d->arch.vmmio.lock);
+d->arch.vmmio.num_entries = 0;
- return 0;
+return 0;
}
/*
diff --git a/xen/include/asm-arm/mmio.h b/xen/include/asm-arm/mmio.h
index da1cc2e..32f10f2 100644
--- a/xen/include/asm-arm/mmio.h
+++ b/xen/include/asm-arm/mmio.h
@@ -20,6 +20,7 @@
#define __ASM_ARM_MMIO_H__
#include
+#include
#include
#include
@@ -51,7 +52,7 @@ struct mmio_handler {
struct vmmio {
int num_entries;
-spinlock_t lock;
+rwlock_t lock;
struct mmio_handler handlers[MAX_IO_HANDLER];
};
--
1.9.1
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] xen/arm: gic-v3: No need to sort the Redistributor regions
On Tue, 28 Jun 2016, Julien Grall wrote:
> The sorting was required by the vGIC emulation until commit
> 9b9d51e98edb8c5c731e2d06dfad3633053d88a4 "xen/arm: vgic-v3:
> Correctly retrieve the vCPU associated to a re-distributor".
>
> Furthermore, the code is buggy because both local variables 'l' and 'r'
> point to the same region.
>
> So drop the code which sort the Redistributors array.
>
> Reported-by: Shanker Donthineni
> Signed-off-by: Julien Grall
Acked-by: Stefano Stabellini
> ---
> Changes in v2:
> - Fix compilation with ACPI
> ---
> xen/arch/arm/gic-v3.c | 14 --
> 1 file changed, 14 deletions(-)
>
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index dfc62e8..b8a4bde 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -1134,14 +1134,6 @@ static const hw_irq_controller gicv3_guest_irq_type = {
> .set_affinity = gicv3_irq_set_affinity,
> };
>
> -static int __init cmp_rdist(const void *a, const void *b)
> -{
> -const struct rdist_region *l = a, *r = a;
> -
> -/* We assume that re-distributor regions can never overlap */
> -return ( l->base < r->base) ? -1 : 0;
> -}
> -
> static paddr_t __initdata dbase = INVALID_PADDR;
> static paddr_t __initdata vbase = INVALID_PADDR, vsize = 0;
> static paddr_t __initdata cbase = INVALID_PADDR, csize = 0;
> @@ -1210,9 +1202,6 @@ static void __init gicv3_dt_init(void)
> rdist_regs[i].size = rdist_size;
> }
>
> -/* The vGIC code requires the region to be sorted */
> -sort(rdist_regs, gicv3.rdist_count, sizeof(*rdist_regs), cmp_rdist,
> NULL);
> -
> if ( !dt_property_read_u32(node, "redistributor-stride",
> &gicv3.rdist_stride) )
> gicv3.rdist_stride = 0;
>
> @@ -1455,9 +1444,6 @@ static void __init gicv3_acpi_init(void)
> rdist_regs[i].size = gic_rdist->length;
> }
>
> -/* The vGIC code requires the region to be sorted */
> -sort(rdist_regs, gicv3.rdist_count, sizeof(*rdist_regs), cmp_rdist,
> NULL);
> -
> gicv3.rdist_regions= rdist_regs;
>
> /* Collect CPU base addresses */
> --
> 1.9.1
>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen/arm: io: Protect the handlers with a read-write lock
On Tue, 28 Jun 2016, Julien Grall wrote:
> Currently, accessing the I/O handlers does not require to take a lock
> because new handlers are always added at the end of the array. In a
> follow-up patch, this array will be sort to optimize the look up.
>
> Given that most of the time the I/O handlers will not be modify,
> using a spinlock will add contention when multiple vCPU are accessing
> the emulated MMIOs. So use a read-write lock to protected the handlers.
>
> Finally, take the opportunity to re-indent correctly domain_io_init.
>
> Signed-off-by: Julien Grall
I would appreciate if you could avoid mixing indentation changes with
other changes in the future.
Reviewed-by: Stefano Stabellini
I'll commit.
> xen/arch/arm/io.c | 47
> +++---
> xen/include/asm-arm/mmio.h | 3 ++-
> 2 files changed, 30 insertions(+), 20 deletions(-)
>
> diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c
> index 0156755..5a96836 100644
> --- a/xen/arch/arm/io.c
> +++ b/xen/arch/arm/io.c
> @@ -70,23 +70,39 @@ static int handle_write(const struct mmio_handler
> *handler, struct vcpu *v,
> handler->priv);
> }
>
> -int handle_mmio(mmio_info_t *info)
> +static const struct mmio_handler *find_mmio_handler(struct domain *d,
> +paddr_t gpa)
> {
> -struct vcpu *v = current;
> -int i;
> -const struct mmio_handler *handler = NULL;
> -const struct vmmio *vmmio = &v->domain->arch.vmmio;
> +const struct mmio_handler *handler;
> +unsigned int i;
> +struct vmmio *vmmio = &d->arch.vmmio;
> +
> +read_lock(&vmmio->lock);
>
> for ( i = 0; i < vmmio->num_entries; i++ )
> {
> handler = &vmmio->handlers[i];
>
> -if ( (info->gpa >= handler->addr) &&
> - (info->gpa < (handler->addr + handler->size)) )
> +if ( (gpa >= handler->addr) &&
> + (gpa < (handler->addr + handler->size)) )
> break;
> }
>
> if ( i == vmmio->num_entries )
> +handler = NULL;
> +
> +read_unlock(&vmmio->lock);
> +
> +return handler;
> +}
> +
> +int handle_mmio(mmio_info_t *info)
> +{
> +struct vcpu *v = current;
> +const struct mmio_handler *handler = NULL;
> +
> +handler = find_mmio_handler(v->domain, info->gpa);
> +if ( !handler )
> return 0;
>
> if ( info->dabt.write )
> @@ -104,7 +120,7 @@ void register_mmio_handler(struct domain *d,
>
> BUG_ON(vmmio->num_entries >= MAX_IO_HANDLER);
>
> -spin_lock(&vmmio->lock);
> +write_lock(&vmmio->lock);
>
> handler = &vmmio->handlers[vmmio->num_entries];
>
> @@ -113,24 +129,17 @@ void register_mmio_handler(struct domain *d,
> handler->size = size;
> handler->priv = priv;
>
> -/*
> - * handle_mmio is not using the lock to avoid contention.
> - * Make sure the other processors see the new handler before
> - * updating the number of entries
> - */
> -dsb(ish);
> -
> vmmio->num_entries++;
>
> -spin_unlock(&vmmio->lock);
> +write_unlock(&vmmio->lock);
> }
>
> int domain_io_init(struct domain *d)
> {
> - spin_lock_init(&d->arch.vmmio.lock);
> - d->arch.vmmio.num_entries = 0;
> +rwlock_init(&d->arch.vmmio.lock);
> +d->arch.vmmio.num_entries = 0;
>
> - return 0;
> +return 0;
> }
>
> /*
> diff --git a/xen/include/asm-arm/mmio.h b/xen/include/asm-arm/mmio.h
> index da1cc2e..32f10f2 100644
> --- a/xen/include/asm-arm/mmio.h
> +++ b/xen/include/asm-arm/mmio.h
> @@ -20,6 +20,7 @@
> #define __ASM_ARM_MMIO_H__
>
> #include
> +#include
> #include
> #include
>
> @@ -51,7 +52,7 @@ struct mmio_handler {
>
> struct vmmio {
> int num_entries;
> -spinlock_t lock;
> +rwlock_t lock;
> struct mmio_handler handlers[MAX_IO_HANDLER];
> };
>
> --
> 1.9.1
>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen/arm: map_dev_mmio_region: The iomem permission check should be done on MFN
On Wed, 15 Jun 2016, Shannon Zhao wrote:
> Hi Julien,
>
> On 2016/6/14 19:50, Julien Grall wrote:
> > The helper iomem_access_permitted expects MFNs in parameters and not
> > GNFs. Thankfully only the hardware domain can call this function and
> > it will always be with GFNS == MFNs for now.
> >
> > Also, fix the printf to use the MFN range and not the GFN one.
> >
> > Signed-off-by: Julien Grall
> > Cc: Shannon Zhao
> >
> Reviewed-by: Shannon Zhao
Shannon, thanks for your help reviewing this.
Acked-by: Stefano Stabellini
I'll commit.
> > ---
> > This patch is a good candidate to backport to Xen 4.7. Without
> > it, the hardware domain can map any MMIO because the permission
> > check is done on the GPFNs and not the MNFs.
> > ---
> > xen/arch/arm/p2m.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> > index 6a19c57..4c6547d 100644
> > --- a/xen/arch/arm/p2m.c
> > +++ b/xen/arch/arm/p2m.c
> > @@ -1275,14 +1275,14 @@ int map_dev_mmio_region(struct domain *d,
> > {
> > int res;
> >
> > -if ( !(nr && iomem_access_permitted(d, start_gfn, start_gfn + nr - 1))
> > )
> > +if ( !(nr && iomem_access_permitted(d, mfn, mfn + nr - 1)) )
> > return 0;
> >
> > res = map_mmio_regions(d, start_gfn, nr, mfn);
> > if ( res < 0 )
> > {
> > printk(XENLOG_G_ERR "Unable to map [%#lx - %#lx] in Dom%d\n",
> > - start_gfn, start_gfn + nr - 1, d->domain_id);
> > + mfn, mfn + nr - 1, d->domain_id);
> > return res;
> > }
> >
> >
>
> --
> Shannon
>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 3/7] hotplug/FreeBSD: honour XEN_RUN_DIR
Store xldevd.pid under XEN_RUN_DIR. Note that the default location would
change from /var/run to /var/run/xen.
Signed-off-by: Wei Liu
Acked-by: Roger Pau Monné
Acked-by: Ian Jackson
---
Cc: Ian Jackson
Cc: Roger Pau Monné
---
tools/hotplug/FreeBSD/rc.d/xendriverdomain.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/hotplug/FreeBSD/rc.d/xendriverdomain.in
b/tools/hotplug/FreeBSD/rc.d/xendriverdomain.in
index 4063c06..8ece7c3 100644
--- a/tools/hotplug/FreeBSD/rc.d/xendriverdomain.in
+++ b/tools/hotplug/FreeBSD/rc.d/xendriverdomain.in
@@ -18,7 +18,7 @@ start_cmd="xendriverdomain_startcmd"
stop_cmd="xendriverdomain_stop"
extra_commands=""
-XLDEVD_PIDFILE="/var/run/xldevd.pid"
+XLDEVD_PIDFILE="@XEN_RUN_DIR@/xldevd.pid"
xendriverdomain_precmd()
{
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 2/7] tools/helper: honour XEN_RUN_DIR in init-xenstore-domain.c
Place the PID file under XEN_RUN_DIR. Note that this change the default
location from /var/run to /var/run/xen.
Generate a _paths.h as that is required to make this change work.
Signed-off-by: Wei Liu
---
Cc: Ian Jackson
v4: fix dependency in Makefile
v3: new
---
.gitignore | 1 +
tools/helpers/Makefile | 7 ++-
tools/helpers/init-xenstore-domain.c | 8 +---
3 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/.gitignore b/.gitignore
index e019f2e..d4ffaa6 100644
--- a/.gitignore
+++ b/.gitignore
@@ -145,6 +145,7 @@ tools/flask/utils/flask-loadpolicy
tools/flask/utils/flask-setenforce
tools/flask/utils/flask-set-bool
tools/flask/utils/flask-label-pci
+tools/helpers/_paths.h
tools/helpers/init-xenstore-domain
tools/helpers/xen-init-dom0
tools/hotplug/common/hotplugpath.sh
diff --git a/tools/helpers/Makefile b/tools/helpers/Makefile
index a05a368..5017350 100644
--- a/tools/helpers/Makefile
+++ b/tools/helpers/Makefile
@@ -28,6 +28,8 @@ all: $(PROGS)
xen-init-dom0: $(XEN_INIT_DOM0_OBJS)
$(CC) $(LDFLAGS) -o $@ $(XEN_INIT_DOM0_OBJS) $(LDLIBS_libxentoollog)
$(LDLIBS_libxenstore) $(LDLIBS_libxenlight) $(APPEND_LDFLAGS)
+$(INIT_XENSTORE_DOMAIN_OBJS): _paths.h
+
init-xenstore-domain: $(INIT_XENSTORE_DOMAIN_OBJS)
$(CC) $(LDFLAGS) -o $@ $(INIT_XENSTORE_DOMAIN_OBJS)
$(LDLIBS_libxentoollog) $(LDLIBS_libxenstore) $(LDLIBS_libxenctrl)
$(LDLIBS_libxenguest) $(LDLIBS_libxenlight) $(APPEND_LDFLAGS)
@@ -41,6 +43,9 @@ endif
.PHONY: clean
clean:
- $(RM) -f *.o $(PROGS) $(DEPS)
+ $(RM) -f *.o $(PROGS) $(DEPS) _paths.h
distclean: clean
+
+genpath-target = $(call buildmakevars2header,_paths.h)
+$(eval $(genpath-target))
diff --git a/tools/helpers/init-xenstore-domain.c
b/tools/helpers/init-xenstore-domain.c
index 909542b..53b4b01 100644
--- a/tools/helpers/init-xenstore-domain.c
+++ b/tools/helpers/init-xenstore-domain.c
@@ -14,6 +14,7 @@
#include
#include "init-dom-json.h"
+#include "_paths.h"
static uint32_t domid = ~0;
static char *kernel;
@@ -316,10 +317,10 @@ int main(int argc, char** argv)
do_xs_write_dom(xsh, "memory/static-max", buf);
xs_close(xsh);
-fd = creat("/var/run/xenstored.pid", 0666);
+fd = creat(XEN_RUN_DIR "/xenstored.pid", 0666);
if ( fd < 0 )
{
-fprintf(stderr, "Creating /var/run/xenstored.pid failed\n");
+fprintf(stderr, "Creating " XEN_RUN_DIR "/xenstored.pid failed\n");
return 3;
}
rv = snprintf(buf, 16, "domid:%d\n", domid);
@@ -327,7 +328,8 @@ int main(int argc, char** argv)
close(fd);
if ( rv < 0 )
{
-fprintf(stderr, "Writing domid to /var/run/xenstored.pid failed\n");
+fprintf(stderr,
+"Writing domid to " XEN_RUN_DIR "/xenstored.pid failed\n");
return 3;
}
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 5/7] hotplug/Linux: honour XEN_RUN_DIR
Store various PID files under XEN_RUN_DIR. Note that this change the
default location from /var/run to /var/run/xen.
Signed-off-by: Wei Liu
Acked-by: Roger Pau Monné
Acked-by: Ian Jackson
---
Cc: Ian Jackson
Cc: Roger Pau Monné
---
tools/hotplug/Linux/init.d/xencommons.in | 6 +++---
tools/hotplug/Linux/init.d/xendriverdomain.in | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/hotplug/Linux/init.d/xencommons.in
b/tools/hotplug/Linux/init.d/xencommons.in
index 7b69fc2..2d8f30b 100644
--- a/tools/hotplug/Linux/init.d/xencommons.in
+++ b/tools/hotplug/Linux/init.d/xencommons.in
@@ -27,8 +27,8 @@ xencommons_config=@CONFIG_DIR@/@CONFIG_LEAF_DIR@
test -f $xencommons_config/xencommons && . $xencommons_config/xencommons
-XENCONSOLED_PIDFILE=/var/run/xenconsoled.pid
-QEMU_PIDFILE=/var/run/qemu-dom0.pid
+XENCONSOLED_PIDFILE=@XEN_RUN_DIR@/xenconsoled.pid
+QEMU_PIDFILE=@XEN_RUN_DIR@/qemu-dom0.pid
shopt -s extglob
# not running in Xen dom0 or domU
@@ -70,7 +70,7 @@ do_start () {
if [ -n "$XENSTORED" ] ; then
echo -n Starting $XENSTORED...
- $XENSTORED --pid-file /var/run/xenstored.pid $XENSTORED_ARGS
+ $XENSTORED --pid-file @XEN_RUN_DIR@/xenstored.pid
$XENSTORED_ARGS
else
echo "No xenstored found"
exit 1
diff --git a/tools/hotplug/Linux/init.d/xendriverdomain.in
b/tools/hotplug/Linux/init.d/xendriverdomain.in
index 8d4592a..c63060f 100644
--- a/tools/hotplug/Linux/init.d/xendriverdomain.in
+++ b/tools/hotplug/Linux/init.d/xendriverdomain.in
@@ -24,7 +24,7 @@ xendriverdomain_config=@CONFIG_DIR@/@CONFIG_LEAF_DIR@
test -f $xendriverdomain_config/xendriverdomain && .
$xendriverdomain_config/xendriverdomain
-XLDEVD_PIDFILE=/var/run/xldevd.pid
+XLDEVD_PIDFILE=@XEN_RUN_DIR@/xldevd.pid
# not running in Xen dom0 or domU
if ! test -d /proc/xen ; then
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 1/7] xenconsoled: honour XEN_RUN_DIR
Place the PID file under XEN_RUN_DIR by default. Note this change the
default location from /var/run to /var/run/xen.
Signed-off-by: Wei Liu
---
Cc: Ian Jackson
---
tools/console/daemon/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/console/daemon/main.c b/tools/console/daemon/main.c
index 20e3513..806d2fd 100644
--- a/tools/console/daemon/main.c
+++ b/tools/console/daemon/main.c
@@ -193,7 +193,7 @@ int main(int argc, char **argv)
increase_fd_limit();
if (!is_interactive) {
- daemonize(pidfile ? pidfile : "/var/run/xenconsoled.pid");
+ daemonize(pidfile ? pidfile : XEN_RUN_DIR "/xenconsoled.pid");
}
if (!xen_setup())
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 0/7] Remove hard-coded /var/run in tools
Wei Liu (7): xenconsoled: honour XEN_RUN_DIR tools/helper: honour XEN_RUN_DIR in init-xenstore-domain.c hotplug/FreeBSD: honour XEN_RUN_DIR hotplug/NetBSD: honour XEN_RUN_DIR hotplug/Linux: honour XEN_RUN_DIR libxenstat: honour XEN_RUN_DIR oxenstored: honour XEN_RUN_DIR .gitignore| 2 ++ tools/console/daemon/main.c | 2 +- tools/helpers/Makefile| 7 ++- tools/helpers/init-xenstore-domain.c | 8 +--- tools/hotplug/FreeBSD/rc.d/xendriverdomain.in | 2 +- tools/hotplug/Linux/init.d/xencommons.in | 6 +++--- tools/hotplug/Linux/init.d/xendriverdomain.in | 2 +- tools/hotplug/NetBSD/rc.d/xendriverdomain.in | 2 +- tools/ocaml/xenstored/xenstored.ml| 2 +- tools/xenstat/libxenstat/Makefile | 7 ++- tools/xenstat/libxenstat/src/xenstat_qmp.c| 3 ++- 11 files changed, 29 insertions(+), 14 deletions(-) -- 2.1.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 7/7] oxenstored: honour XEN_RUN_DIR
Move default the pid file under XEN_RUN_DIR. Note that it changes the location from /var/run to /var/run/xen. Signed-off-by: Wei Liu Acked-by: David Scott --- Cc: Ian Jackson Not a backport candidate. --- tools/ocaml/xenstored/xenstored.ml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/ocaml/xenstored/xenstored.ml b/tools/ocaml/xenstored/xenstored.ml index 30570ed..7ea4026 100644 --- a/tools/ocaml/xenstored/xenstored.ml +++ b/tools/ocaml/xenstored/xenstored.ml @@ -81,7 +81,7 @@ let config_filename cf = | Some name -> name | None -> Define.default_config_dir ^ "/oxenstored.conf" -let default_pidfile = "/var/run/xenstored.pid" +let default_pidfile = Paths.xen_run_dir ^ "/xenstored.pid" let ring_scan_interval = ref 20 -- 2.1.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 4/7] hotplug/NetBSD: honour XEN_RUN_DIR
Store xldevd.pid under XEN_RUN_DIR. Note that this will change the
default location from /var/run to /var/run/xen.
Signed-off-by: Wei Liu
Acked-by: Roger Pau Monné
Acked-by: Ian Jackson
---
Cc: Ian Jackson
Cc: Roger Pau Monné
---
tools/hotplug/NetBSD/rc.d/xendriverdomain.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/hotplug/NetBSD/rc.d/xendriverdomain.in
b/tools/hotplug/NetBSD/rc.d/xendriverdomain.in
index 5062a71..f47b0b1 100644
--- a/tools/hotplug/NetBSD/rc.d/xendriverdomain.in
+++ b/tools/hotplug/NetBSD/rc.d/xendriverdomain.in
@@ -19,7 +19,7 @@ start_cmd="xendriverdomain_startcmd"
stop_cmd="xendriverdomain_stop"
extra_commands=""
-XLDEVD_PIDFILE="/var/run/xldevd.pid"
+XLDEVD_PIDFILE="@XEN_RUN_DIR@/xldevd.pid"
xendriverdomain_precmd()
{
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 6/7] libxenstat: honour XEN_RUN_DIR
This is because libxl uses XEN_RUN_DIR to generate the socket path for libxenstat while libxenstat itself uses hard-coded path, which is not necessarily the same path as XEN_RUN_DIR. The default configuration happened to work because XEN_RUN_DIR defaulted to /var/run/xen, which matched the hard-coded path. We should make libxenstat use XEN_RUN_DIR so that it works with non-default configuration. Generate a _paths.h because it is required to make this change work. Signed-off-by: Wei Liu --- Cc: Ian Jackson Backport candidate. --- .gitignore | 1 + tools/xenstat/libxenstat/Makefile | 7 ++- tools/xenstat/libxenstat/src/xenstat_qmp.c | 3 ++- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index d4ffaa6..9b8dece 100644 --- a/.gitignore +++ b/.gitignore @@ -220,6 +220,7 @@ tools/xenmon/xentrace_setmask tools/xenmon/xenbaked tools/xenpaging/xenpaging tools/xenpmd/xenpmd +tools/xenstat/libxenstat/src/_paths.h tools/xenstat/xentop/xentop tools/xenstore/xenstore tools/xenstore/xenstore-chmod diff --git a/tools/xenstat/libxenstat/Makefile b/tools/xenstat/libxenstat/Makefile index 850d24a..213d998 100644 --- a/tools/xenstat/libxenstat/Makefile +++ b/tools/xenstat/libxenstat/Makefile @@ -40,6 +40,8 @@ LDLIBS-$(CONFIG_SunOS) += -lkstat .PHONY: all all: $(LIB) $(SHLIB) $(SHLIB_LINKS) +$(OBJECTS-y): src/_paths.h + $(LIB): $(OBJECTS-y) $(AR) rc $@ $^ $(RANLIB) $@ @@ -135,9 +137,12 @@ endif .PHONY: clean clean: rm -f $(LIB) $(SHLIB) $(SHLIB_LINKS) $(OBJECTS-y) \ - $(BINDINGS) $(BINDINGSRC) $(DEPS) + $(BINDINGS) $(BINDINGSRC) $(DEPS) src/_paths.h .PHONY: distclean distclean: clean -include $(DEPS) + +genpath-target = $(call buildmakevars2header,src/_paths.h) +$(eval $(genpath-target)) diff --git a/tools/xenstat/libxenstat/src/xenstat_qmp.c b/tools/xenstat/libxenstat/src/xenstat_qmp.c index c12929a..a87c937 100644 --- a/tools/xenstat/libxenstat/src/xenstat_qmp.c +++ b/tools/xenstat/libxenstat/src/xenstat_qmp.c @@ -23,6 +23,7 @@ #include #include "xenstat_priv.h" +#include "_paths.h" #ifdef HAVE_YAJL_YAJL_VERSION_H # include @@ -398,7 +399,7 @@ static void read_attributes_qdisk_dom(xenstat_node *node, domid_t domain) free(val); /* Connect to this VMs QMP socket */ - snprintf(path, sizeof(path), "/var/run/xen/qmp-libxenstat-%i", domain); + snprintf(path, sizeof(path), XEN_RUN_DIR "/qmp-libxenstat-%i", domain); if ((qfd = qmp_connect(path)) < 0) return; -- 2.1.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv1] xen/evtchn: add IOCTL_EVTCHN_RESTRICT
On 07/11/2016 12:44 PM, David Vrabel wrote:
> On 11/07/16 17:33, Andrew Cooper wrote:
>> On 11/07/16 17:15, David Vrabel wrote:
>>> On 11/07/16 16:31, Boris Ostrovsky wrote:
On 07/11/2016 10:57 AM, David Vrabel wrote:
> diff --git a/include/uapi/xen/evtchn.h b/include/uapi/xen/evtchn.h
> index 14e833ee4..f057b53 100644
> --- a/include/uapi/xen/evtchn.h
> +++ b/include/uapi/xen/evtchn.h
> @@ -85,4 +85,19 @@ struct ioctl_evtchn_notify {
> #define IOCTL_EVTCHN_RESET \
> _IOC(_IOC_NONE, 'E', 5, 0)
>
> +/*
> + * Restrict this file descriptor so that it can only be used to bind
> + * new interdomain events from one domain.
> + *
> + * Once a file descriptor has been restricted it cannot be
> + * de-restricted, and must be closed and re-opened. Event channels
> + * which were bound before restricting remain bound afterwards, and
> + * can be notified as usual.
> + */
> +#define IOCTL_EVTCHN_RESTRICT_DOMID \
> + _IOC(_IOC_NONE, 'E', 100, sizeof(struct ioctl_evtchn_restrict_domid))
Is there a reason why you picked 100 and not 6?
>>> Because we've had this patch for years in xenserver like this and I
>>> didn't see any need to change the ABI. But if it's preferred I can make
>>> this 6 (and manage the transition internally).
>> This should become 6, and we manage the transition. It is not like its
>> hard to manage.
> Ok.
With that
Reviewed-by: Boris Ostrovsky
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv1] xen/evtchn: add IOCTL_EVTCHN_RESTRICT
On 11/07/16 17:33, Andrew Cooper wrote:
> On 11/07/16 17:15, David Vrabel wrote:
>> On 11/07/16 16:31, Boris Ostrovsky wrote:
>>> On 07/11/2016 10:57 AM, David Vrabel wrote:
diff --git a/include/uapi/xen/evtchn.h b/include/uapi/xen/evtchn.h
index 14e833ee4..f057b53 100644
--- a/include/uapi/xen/evtchn.h
+++ b/include/uapi/xen/evtchn.h
@@ -85,4 +85,19 @@ struct ioctl_evtchn_notify {
#define IOCTL_EVTCHN_RESET\
_IOC(_IOC_NONE, 'E', 5, 0)
+/*
+ * Restrict this file descriptor so that it can only be used to bind
+ * new interdomain events from one domain.
+ *
+ * Once a file descriptor has been restricted it cannot be
+ * de-restricted, and must be closed and re-opened. Event channels
+ * which were bound before restricting remain bound afterwards, and
+ * can be notified as usual.
+ */
+#define IOCTL_EVTCHN_RESTRICT_DOMID \
+ _IOC(_IOC_NONE, 'E', 100, sizeof(struct ioctl_evtchn_restrict_domid))
>>> Is there a reason why you picked 100 and not 6?
>> Because we've had this patch for years in xenserver like this and I
>> didn't see any need to change the ABI. But if it's preferred I can make
>> this 6 (and manage the transition internally).
>
> This should become 6, and we manage the transition. It is not like its
> hard to manage.
Ok.
David
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 03/16] x86/monitor: mechanical renames
On Sat, Jul 9, 2016 at 12:46 PM, Corneliu ZUZU wrote: > On 7/9/2016 9:10 PM, Tamas K Lengyel wrote: >> >> On Fri, Jul 8, 2016 at 10:13 PM, Corneliu ZUZU >> wrote: >>> >>> Arch-specific vm-event functions in x86/vm_event.h -e.g. >>> vm_event_init_domain()- >>> don't have an 'arch_' prefix. Apply the same rule for monitor functions - >>> originally the only two monitor functions that had an 'arch_' prefix were >>> arch_monitor_domctl_event() and arch_monitor_domctl_op(), but I gave them >>> that >>> prefix because -they had a counterpart function in common code-, that >>> being >>> monitor_domctl(). >> >> This should actually be the other way around - ie adding the arch_ >> prefix to vm_event functions that lack it. > > > Given that the majority of the arch-specific functions called from > common-code don't have an 'arch_' prefix unless they have a common > counterpart, I was guessing that was the rule. It made sense in my head > since I saw in that the intention of avoiding naming conflicts (i.e you > can't have monitor_domctl() both on the common-side and on the arch-side, so > prepend 'arch_' to the latter). I noticed you guys also 'skip' the prefix > when sending patches, so that reinforced my assumption. > >> Having the arch_ prefix is >> helpful to know that the function is dealing with the arch specific >> structs and not common. > > > Personally I don't see much use in 'knowing that the function is dealing > with the arch structs' from the call-site and you can tell that from the > implementation-site just by looking at the path of its source file. Also, > the code is pretty much localized in the arch directory anyway so usually > one wouldn't have to go back and forth between common and arch that often. > What really bothers me though is always having to read 'arch_' when spelling > a function-name and also that it makes the function name longer without much > benefit. Your suggestion of adding it to pretty much all functions that make > up the interface to common just adds to that headache. :-D > >> Similarly that's why we have the hvm_ prefix >> for functions in hvm/monitor. > > > 'hvm_' doesn't seem to me more special than 'monitor_', for instance, but > maybe that's just me. > >>> Let this also be the rule for future 'arch_' functions additions, and >>> with this >>> patch remove the 'arch_' prefix from the monitor functions that don't >>> have a >>> counterpart in common-code (all but those 2 aforementioned). >> >> Even if there are no common counter-parts to the function, the arch_ >> prefix should remain, so I won't be able to ack this patch. >> >> Tamas > > > Having said the above, are you still of the same opinion? Yes, I am. While it's not a hard rule to always apply these prefix, it does make sense to have them so I don't see benefit in removing the existing prefixes. Adding arch_ prefix to the ones that don't already have one is optional, I was just pointing out that if you really feel like standardizing the naming convention, that's where I would like things to move towards to. Tamas ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 4/5] libxl: update vcpus bitmap in retrieved guest config
Wei Liu writes ("Re: [PATCH v4 4/5] libxl: update vcpus bitmap in retrieved
guest config"):
> The hunk is (on top of this patch) to use libxl__xs_read_checked
Errr. Sorry for not spotting that in my review.
> Is it ok to keep your ack?
Yes, thanks.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 11/16] x86/monitor: fix: treat -monitor- properly, as a subsys of the vm-event subsys
>>> diff --git a/xen/include/asm-arm/monitor.h
>>> b/xen/include/asm-arm/monitor.h
>>> index 9a9734a..7ef30f1 100644
>>
>> [snip]
>
>
> I keep seeing '[snip]' lately but I don't know what it means.
Placeholder for "I've cut some of your patch content here to shorten
the message I'm sending".
>
>>
>>> diff --git a/xen/include/xen/monitor.h b/xen/include/xen/monitor.h
>>> index 2171d04..605caf0 100644
>>> --- a/xen/include/xen/monitor.h
>>> +++ b/xen/include/xen/monitor.h
>>> @@ -22,12 +22,15 @@
>>> #ifndef __XEN_MONITOR_H__
>>> #define __XEN_MONITOR_H__
>>>
>>> -#include
>>> -
>>> -struct domain;
>>> -struct xen_domctl_monitor_op;
>>> +#include
>>>
>>> int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *op);
>>> +
>>> +static inline bool_t monitor_domain_initialised(const struct domain *d)
>>> +{
>>> +return d->monitor.initialised;
>>
>> This should be !!d->monitor.initialised.
>
>
> It's the value of a bit, thus should be 0 or 1. Am I missing something?
Using a bitfield is effectively doing a bitmask for the bit(s).
However, returning the value after applying a bitmask is not
necessarily 0/1 (ie. bool_t). For example I ran into problems with
this in the past (see
https://lists.xen.org/archives/html/xen-devel/2015-08/msg01948.html).
Tamas
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv1] xen/evtchn: add IOCTL_EVTCHN_RESTRICT
On 11/07/16 17:15, David Vrabel wrote:
> On 11/07/16 16:31, Boris Ostrovsky wrote:
>> On 07/11/2016 10:57 AM, David Vrabel wrote:
>>> diff --git a/include/uapi/xen/evtchn.h b/include/uapi/xen/evtchn.h
>>> index 14e833ee4..f057b53 100644
>>> --- a/include/uapi/xen/evtchn.h
>>> +++ b/include/uapi/xen/evtchn.h
>>> @@ -85,4 +85,19 @@ struct ioctl_evtchn_notify {
>>> #define IOCTL_EVTCHN_RESET \
>>> _IOC(_IOC_NONE, 'E', 5, 0)
>>>
>>> +/*
>>> + * Restrict this file descriptor so that it can only be used to bind
>>> + * new interdomain events from one domain.
>>> + *
>>> + * Once a file descriptor has been restricted it cannot be
>>> + * de-restricted, and must be closed and re-opened. Event channels
>>> + * which were bound before restricting remain bound afterwards, and
>>> + * can be notified as usual.
>>> + */
>>> +#define IOCTL_EVTCHN_RESTRICT_DOMID\
>>> + _IOC(_IOC_NONE, 'E', 100, sizeof(struct ioctl_evtchn_restrict_domid))
>> Is there a reason why you picked 100 and not 6?
> Because we've had this patch for years in xenserver like this and I
> didn't see any need to change the ABI. But if it's preferred I can make
> this 6 (and manage the transition internally).
This should become 6, and we manage the transition. It is not like its
hard to manage.
~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 4/5] libxl: update vcpus bitmap in retrieved guest config
On Mon, Jul 11, 2016 at 04:56:02PM +0100, Ian Jackson wrote:
> Wei Liu writes ("[PATCH v4 4/5] libxl: update vcpus bitmap in retrieved guest
> config"):
> > ... because the available vcpu bitmap can change during domain life time
> > due to cpu hotplug and unplug.
> >
> > For QEMU upstream, we interrogate QEMU for the number of vcpus. For
> > others, we look directly into xenstore for information.
> >
> > Reported-by: Jan Beulich
> > Signed-off-by: Wei Liu
>
> Acked-by: Ian Jackson
Sorry I just realise there is one hunk I forgot to commit.
The hunk is (on top of this patch) to use libxl__xs_read_checked
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index e4b0424..e49741d 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -7305,8 +7305,10 @@ static int libxl__update_avail_vcpus_xenstore(libxl__gc
*gc, uint32_t domid,
for (i = 0; i < max_vcpus; i++) {
const char *path = GCSPRINTF("%s/cpu/%u/availability", dompath, i);
-const char *content = libxl__xs_read(gc, XBT_NULL, path);
-if (!strcmp(content, "online"))
+const char *content;
+rc = libxl__xs_read_checked(gc, XBT_NULL, path, &content);
+if (rc) goto out;
+if (content && !strcmp(content, "online"))
libxl_bitmap_set(map, i);
}
Is it ok to keep your ack?
Wei.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6 12/14] xen/arm: p2m: Introduce helpers to insert and remove mapping
Hi,
On 06/07/16 14:01, Julien Grall wrote:
int map_mmio_regions(struct domain *d,
@@ -1189,12 +1207,8 @@ int map_mmio_regions(struct domain *d,
unsigned long nr,
mfn_t mfn)
{
-return apply_p2m_changes(d, INSERT,
- pfn_to_paddr(gfn_x(start_gfn)),
- pfn_to_paddr(gfn_x(start_gfn) + nr),
- pfn_to_paddr(mfn_x(mfn)),
- MATTR_DEV, 0, p2m_mmio_direct,
- d->arch.p2m.default_access);
+return p2m_insert_mapping(d, start_gfn, nr, mfn,
+ MATTR_MEM, p2m_mmio_direct);
This should be MATTR_DEV and not MATTR_MEM. I will fix it in the next
version.
}
Regards,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv1] xen/evtchn: add IOCTL_EVTCHN_RESTRICT
On 11/07/16 16:31, Boris Ostrovsky wrote:
> On 07/11/2016 10:57 AM, David Vrabel wrote:
>> diff --git a/include/uapi/xen/evtchn.h b/include/uapi/xen/evtchn.h
>> index 14e833ee4..f057b53 100644
>> --- a/include/uapi/xen/evtchn.h
>> +++ b/include/uapi/xen/evtchn.h
>> @@ -85,4 +85,19 @@ struct ioctl_evtchn_notify {
>> #define IOCTL_EVTCHN_RESET \
>> _IOC(_IOC_NONE, 'E', 5, 0)
>>
>> +/*
>> + * Restrict this file descriptor so that it can only be used to bind
>> + * new interdomain events from one domain.
>> + *
>> + * Once a file descriptor has been restricted it cannot be
>> + * de-restricted, and must be closed and re-opened. Event channels
>> + * which were bound before restricting remain bound afterwards, and
>> + * can be notified as usual.
>> + */
>> +#define IOCTL_EVTCHN_RESTRICT_DOMID \
>> +_IOC(_IOC_NONE, 'E', 100, sizeof(struct ioctl_evtchn_restrict_domid))
>
> Is there a reason why you picked 100 and not 6?
Because we've had this patch for years in xenserver like this and I
didn't see any need to change the ABI. But if it's preferred I can make
this 6 (and manage the transition internally).
David
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v1 02/20] acpi/hvmloader: Move acpi_info initialization out of ACPI code
On 07/11/2016 11:38 AM, Ian Jackson wrote:
> Boris Ostrovsky writes ("Re: [Xen-devel] [PATCH v1 02/20] acpi/hvmloader:
> Move acpi_info initialization out of ACPI code"):
>> But do we need to look at who touched the file or who is explicitly
>> listed as copyright holder (in files' headers)?
> You need to look at the history.
>
> Sadly, we don't have any means of keeping the copyright headers up to
> date with lists of contributors.
>
>> So I did (because I thought only Signed-off might important, is it?)
> No. In general I think From: is probably more relevant but I would
> include both From: and S-o-b:.
>
> If there are edge cases that crop up only once or twice they could be
> excluded.
OK, then with contribution count:
ostr@workbase> git log . | egrep "Signed|From" | sed -e 's/.*@/ /g' |
sort | uniq -c
1 citrix.com
73 citrix.com>
1 debian.org>
5 eu.citrix.com>
18 intel.com>
2 jp.fujitsu.com>
1 net-space.pl>
1 novell.com>
1 oracle.com>
1 redhat.com>
1 sun.com>
8 suse.com>
1 us.ibm.com>
6 verge.net.au>
4 xen.org>
25 xensource.com>
debian.com is a single trivial patch:
http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=83f34fdcdd26c3dcc793c571e7b75c705bd92e7a
fujitsu is one trivial and one somewhat less trivial
http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=e451db15ef6198f5d21b84618c833ac276087d70
http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=ab438874b6a8ae955b337c36e7b3204e29b8d407
net-space.pl is Daniel Kiper (who is at Oracle now)
redhat is one simple patch (that has been reverted):
http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=e4fd0475a08fda414da27c4e57b568f147cfc07e
ibm is one significant patch:
http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=9fd9787b0e7995ac5f2da504b92723c24d6a3737
verge is a bunch of patches by Simon Horman who probably still is at
that address (I see a message from him on the list from lat December)
xensource and xen.org tags are all from Keir.
So it seems that we need to get permission to convert GPLv2 to LGPL from
Citrix
Intel
Suse/Novell
Oracle/Sun
Simon Horman/Verge
Keir (his commits signed from @xen.org are all from 2011, I don't know
whether that was still while he was at Citrix/Xensource)
+maybe IBM.
-boris
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 5/5] libxl: only issue cpu-add call to QEMU for not present CPU
Wei Liu writes ("[PATCH v4 5/5] libxl: only issue cpu-add call to QEMU for not
present CPU"):
> Calculate the final bitmap for CPUs to add to avoid having annoying
> error messages complaining those CPUs are already present. Example
> message is like (wrapped):
>
> libxl: error: libxl_qmp.c:287:qmp_handle_error_response: received an
> error message from QMP server: Unable to add CPU: 0, it already exists
>
> We can also properly handle error from QMP now.
Acked-by: Ian Jackson
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 4/5] libxl: update vcpus bitmap in retrieved guest config
Wei Liu writes ("[PATCH v4 4/5] libxl: update vcpus bitmap in retrieved guest
config"):
> ... because the available vcpu bitmap can change during domain life time
> due to cpu hotplug and unplug.
>
> For QEMU upstream, we interrogate QEMU for the number of vcpus. For
> others, we look directly into xenstore for information.
>
> Reported-by: Jan Beulich
> Signed-off-by: Wei Liu
Acked-by: Ian Jackson
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 2/5] libxl: libxl_domain_need_memory shouldn't modify b_info
Wei Liu writes ("[PATCH v4 2/5] libxl: libxl_domain_need_memory shouldn't
modify b_info"):
> This function is used to return the memory needed for a guest. It's not
> in a position to modify the b_info passed in (note the _setdefault
> function).
>
> Constify the passed in b_info, use a copy to do the calculation. Mark
> the change in API in libxl.h.
Acked-by: Ian Jackson
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 1/5] libxl: constify copy and length calculation functions
Wei Liu writes ("[PATCH v4 1/5] libxl: constify copy and length calculation
functions"):
> These functions are not supposed to modify the passed in parameters.
> Reflect that in function declarations.
Acked-by: Ian Jackson
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v4 4/5] libxl: update vcpus bitmap in retrieved guest config
... because the available vcpu bitmap can change during domain life time
due to cpu hotplug and unplug.
For QEMU upstream, we interrogate QEMU for the number of vcpus. For
others, we look directly into xenstore for information.
Reported-by: Jan Beulich
Signed-off-by: Wei Liu
---
Cc: Ian Jackson
Cc: Anthony PERARD
v4:
1. Use libxl__device_model_version_running
2. Move comment
v3:
1. Fix indentation of abort.
2. Use strcmp instead of strncmp.
---
tools/libxl/libxl.c | 90 +
1 file changed, 90 insertions(+)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 51d202f..3786b09 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -7249,6 +7249,53 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst,
const libxl_mac *src)
(*dst)[i] = (*src)[i];
}
+/* For QEMU upstream we always need to provide the number of cpus present to
+ * QEMU whether they are online or not; otherwise QEMU won't accept the saved
+ * state. See implementation of libxl__qmp_query_cpus.
+ */
+static int libxl__update_avail_vcpus_qmp(libxl__gc *gc, uint32_t domid,
+ unsigned int max_vcpus,
+ libxl_bitmap *map)
+{
+int rc;
+
+rc = libxl__qmp_query_cpus(gc, domid, map);
+if (rc) {
+LOG(ERROR, "fail to get number of cpus for domain %d", domid);
+goto out;
+}
+
+rc = 0;
+out:
+return rc;
+}
+
+static int libxl__update_avail_vcpus_xenstore(libxl__gc *gc, uint32_t domid,
+ unsigned int max_vcpus,
+ libxl_bitmap *map)
+{
+int rc;
+unsigned int i;
+const char *dompath;
+
+dompath = libxl__xs_get_dompath(gc, domid);
+if (!dompath) {
+rc = ERROR_FAIL;
+goto out;
+}
+
+for (i = 0; i < max_vcpus; i++) {
+const char *path = GCSPRINTF("%s/cpu/%u/availability", dompath, i);
+const char *content = libxl__xs_read(gc, XBT_NULL, path);
+if (!strcmp(content, "online"))
+libxl_bitmap_set(map, i);
+}
+
+rc = 0;
+out:
+return rc;
+}
+
int libxl_retrieve_domain_configuration(libxl_ctx *ctx, uint32_t domid,
libxl_domain_config *d_config)
{
@@ -7297,6 +7344,49 @@ int libxl_retrieve_domain_configuration(libxl_ctx *ctx,
uint32_t domid,
libxl_dominfo_dispose(&info);
}
+/* VCPUs */
+{
+libxl_bitmap *map = &d_config->b_info.avail_vcpus;
+unsigned int max_vcpus = d_config->b_info.max_vcpus;
+libxl_device_model_version version;
+
+libxl_bitmap_dispose(map);
+libxl_bitmap_init(map);
+libxl_bitmap_alloc(CTX, map, max_vcpus);
+libxl_bitmap_set_none(map);
+
+switch (d_config->b_info.type) {
+case LIBXL_DOMAIN_TYPE_HVM:
+version = libxl__device_model_version_running(gc, domid);
+assert(version != LIBXL_DEVICE_MODEL_VERSION_UNKNOWN);
+switch (version) {
+case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
+rc = libxl__update_avail_vcpus_qmp(gc, domid,
+ max_vcpus, map);
+break;
+case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
+case LIBXL_DEVICE_MODEL_VERSION_NONE:
+rc = libxl__update_avail_vcpus_xenstore(gc, domid,
+max_vcpus, map);
+break;
+default:
+abort();
+}
+break;
+case LIBXL_DOMAIN_TYPE_PV:
+rc = libxl__update_avail_vcpus_xenstore(gc, domid,
+max_vcpus, map);
+break;
+default:
+abort();
+}
+
+if (rc) {
+LOG(ERROR, "fail to update available cpu map for domain %d",
domid);
+goto out;
+}
+}
+
/* Memory limits:
*
* Currently there are three memory limits:
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v4 1/5] libxl: constify copy and length calculation functions
These functions are not supposed to modify the passed in parameters.
Reflect that in function declarations.
Mark the change in APIs in libxl.h
Signed-off-by: Wei Liu
---
Cc: Ian Jackson
---
tools/libxl/gentypes.py | 4 ++--
tools/libxl/libxl.c | 10 +-
tools/libxl/libxl.h | 23 +++
tools/libxl/libxl_cpuid.c| 4 ++--
tools/libxl/libxl_genid.c| 2 +-
tools/libxl/libxl_internal.h | 2 +-
tools/libxl/libxl_utils.c| 2 +-
tools/libxl/libxl_utils.h| 2 +-
8 files changed, 28 insertions(+), 21 deletions(-)
diff --git a/tools/libxl/gentypes.py b/tools/libxl/gentypes.py
index 00816c0..4ea7091 100644
--- a/tools/libxl/gentypes.py
+++ b/tools/libxl/gentypes.py
@@ -544,7 +544,7 @@ if __name__ == '__main__':
if ty.dispose_fn is not None:
f.write("%svoid %s(%s);\n" % (ty.hidden(), ty.dispose_fn,
ty.make_arg("p")))
if ty.copy_fn is not None:
-f.write("%svoid %s(libxl_ctx *ctx, %s, %s);\n" % (ty.hidden(),
ty.copy_fn,
+f.write("%svoid %s(libxl_ctx *ctx, %s, const %s);\n" %
(ty.hidden(), ty.copy_fn,
ty.make_arg("dst"),
ty.make_arg("src")))
if ty.init_fn is not None:
f.write("%svoid %s(%s);\n" % (ty.hidden(), ty.init_fn,
ty.make_arg("p")))
@@ -649,7 +649,7 @@ if __name__ == '__main__':
f.write("\n")
for ty in [t for t in types if t.copy_fn and t.autogenerate_copy_fn]:
-f.write("void %s(libxl_ctx *ctx, %s, %s)\n" % (ty.copy_fn,
+f.write("void %s(libxl_ctx *ctx, %s, const %s)\n" % (ty.copy_fn,
ty.make_arg("dst",
passby=idl.PASS_BY_REFERENCE),
ty.make_arg("src",
passby=idl.PASS_BY_REFERENCE)))
f.write("{\n")
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 1c81239..0c34d6b 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -234,7 +234,7 @@ void libxl_string_list_dispose(libxl_string_list *psl)
void libxl_string_list_copy(libxl_ctx *ctx,
libxl_string_list *dst,
-libxl_string_list *src)
+const libxl_string_list *src)
{
GC_INIT(ctx);
int i, len;
@@ -266,7 +266,7 @@ int libxl_string_list_length(const libxl_string_list *psl)
return i;
}
-int libxl_key_value_list_length(libxl_key_value_list *pkvl)
+int libxl_key_value_list_length(const libxl_key_value_list *pkvl)
{
int i = 0;
libxl_key_value_list kvl = *pkvl;
@@ -301,7 +301,7 @@ void libxl_key_value_list_dispose(libxl_key_value_list
*pkvl)
void libxl_key_value_list_copy(libxl_ctx *ctx,
libxl_key_value_list *dst,
- libxl_key_value_list *src)
+ const libxl_key_value_list *src)
{
GC_INIT(ctx);
int i, len;
@@ -7227,7 +7227,7 @@ out_err:
}
-void libxl_hwcap_copy(libxl_ctx *ctx,libxl_hwcap *dst, libxl_hwcap *src)
+void libxl_hwcap_copy(libxl_ctx *ctx,libxl_hwcap *dst, const libxl_hwcap *src)
{
int i;
@@ -7235,7 +7235,7 @@ void libxl_hwcap_copy(libxl_ctx *ctx,libxl_hwcap *dst,
libxl_hwcap *src)
(*dst)[i] = (*src)[i];
}
-void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, libxl_mac *src)
+void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, const libxl_mac *src)
{
int i;
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 2c0f868..f2843fd 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -67,6 +67,13 @@
* the same $(XEN_VERSION) (e.g. throughout a major release).
*/
+/* LIBXL_HAVE_CONST_COPY_AND_LENGTH_FUNCTIONS
+ *
+ * If this is defined, the copy functions have constified src parameter and the
+ * length functions accept constified parameter.
+ */
+#define LIBXL_HAVE_CONST_COPY_AND_LENGTH_FUNCTIONS 1
+
/* LIBXL_HAVE_VNUMA
*
* If this is defined the type libxl_vnode_info exists, and a
@@ -839,7 +846,7 @@ typedef uint8_t libxl_mac[6];
#define LIBXL_MAC_FMT "%02hhx:%02hhx:%02hhx:%02hhx:%02hhx:%02hhx"
#define LIBXL_MAC_FMTLEN ((2*6)+5) /* 6 hex bytes plus 5 colons */
#define LIBXL_MAC_BYTES(mac) mac[0], mac[1], mac[2], mac[3], mac[4], mac[5]
-void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, libxl_mac *src);
+void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, const libxl_mac *src);
#if defined(__i386__) || defined(__x86_64__)
/*
@@ -962,17 +969,17 @@ typedef char **libxl_string_list;
void libxl_string_list_dispose(libxl_string_list *sl);
int libxl_string_list_length(const libxl_string_list *sl);
void libxl_string_list_copy(libxl_ctx *ctx, libxl_string_list *dst,
-libxl_string_list *src);
+const libxl_string_list *src);
typedef char **libxl_key_value_list;
void libxl_key_value_list_dispose(libxl_key_value_list *kvl);
-int libxl_key_value_list_length(libxl_key_value_list *kvl);
+int libxl_key
[Xen-devel] [PATCH v4 5/5] libxl: only issue cpu-add call to QEMU for not present CPU
Calculate the final bitmap for CPUs to add to avoid having annoying
error messages complaining those CPUs are already present. Example
message is like (wrapped):
libxl: error: libxl_qmp.c:287:qmp_handle_error_response: received an
error message from QMP server: Unable to add CPU: 0, it already exists
We can also properly handle error from QMP now.
Signed-off-by: Wei Liu
Reviewed-by: Anthony PERARD
---
Cc: Ian Jackson
Cc: Anthony PERARD
v4:
1. update commit log to contain example error message.
v3:
1. Add Anthony's Reviewed-by tag.
---
tools/libxl/libxl.c | 39 +--
1 file changed, 29 insertions(+), 10 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 3786b09..e4b0424 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -5756,19 +5756,38 @@ static int libxl__set_vcpuonline_qmp(libxl__gc *gc,
uint32_t domid,
libxl_bitmap *cpumap,
const libxl_dominfo *info)
{
-int i;
+int i, rc;
+libxl_bitmap current_map, final_map;
+
+libxl_bitmap_init(¤t_map);
+libxl_bitmap_init(&final_map);
+
+libxl_bitmap_alloc(CTX, ¤t_map, info->vcpu_max_id + 1);
+libxl_bitmap_set_none(¤t_map);
+rc = libxl__qmp_query_cpus(gc, domid, ¤t_map);
+if (rc) {
+LOG(ERROR, "failed to query cpus for domain %d", domid);
+goto out;
+}
+
+libxl_bitmap_copy_alloc(CTX, &final_map, cpumap);
-for (i = 0; i <= info->vcpu_max_id; i++) {
-if (libxl_bitmap_test(cpumap, i)) {
-/* Return value is ignore because it does not tell anything useful
- * on the completion of the command.
- * (For instance, "CPU already plugged-in" give the same return
- * value as "command not supported".)
- */
-libxl__qmp_cpu_add(gc, domid, i);
+libxl_for_each_set_bit(i, current_map)
+libxl_bitmap_reset(&final_map, i);
+
+libxl_for_each_set_bit(i, final_map) {
+rc = libxl__qmp_cpu_add(gc, domid, i);
+if (rc) {
+LOG(ERROR, "failed to add cpu %d to domain %d", i, domid);
+goto out;
}
}
-return 0;
+
+rc = 0;
+out:
+libxl_bitmap_dispose(¤t_map);
+libxl_bitmap_dispose(&final_map);
+return rc;
}
int libxl_set_vcpuonline(libxl_ctx *ctx, uint32_t domid, libxl_bitmap *cpumap)
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v4 3/5] libxl: introduce libxl__qmp_query_cpus
It interrogates QEMU for CPUs and update the bitmap accordingly.
Signed-off-by: Wei Liu
Reviewed-by: Dario Faggioli
Reviewed-by: Anthony PERARD
Acked-by: Ian Jackson
---
Cc: Ian Jackson
Cc: Anthony PERARD
v3:
1. Initialise rc in error path.
2. Fix comment in header and a typo in log message.
---
tools/libxl/libxl_internal.h | 3 +++
tools/libxl/libxl_qmp.c | 38 ++
2 files changed, 41 insertions(+)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index de77579..e33c710 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1794,6 +1794,9 @@ _hidden int libxl__qmp_set_global_dirty_log(libxl__gc
*gc, int domid, bool enabl
_hidden int libxl__qmp_insert_cdrom(libxl__gc *gc, int domid, const
libxl_device_disk *disk);
/* Add a virtual CPU */
_hidden int libxl__qmp_cpu_add(libxl__gc *gc, int domid, int index);
+/* Query the bitmap of CPUs */
+_hidden int libxl__qmp_query_cpus(libxl__gc *gc, int domid,
+ libxl_bitmap *map);
/* Start NBD server */
_hidden int libxl__qmp_nbd_server_start(libxl__gc *gc, int domid,
const char *host, const char *port);
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 3eb279a..63c49c5 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -979,6 +979,44 @@ int libxl__qmp_cpu_add(libxl__gc *gc, int domid, int idx)
return qmp_run_command(gc, domid, "cpu-add", args, NULL, NULL);
}
+static int query_cpus_callback(libxl__qmp_handler *qmp,
+ const libxl__json_object *response,
+ void *opaque)
+{
+libxl_bitmap *map = opaque;
+unsigned int i;
+const libxl__json_object *cpu = NULL;
+int rc;
+GC_INIT(qmp->ctx);
+
+libxl_bitmap_set_none(map);
+for (i = 0; (cpu = libxl__json_array_get(response, i)); i++) {
+unsigned int idx;
+const libxl__json_object *o;
+
+o = libxl__json_map_get("CPU", cpu, JSON_INTEGER);
+if (!o) {
+LOG(ERROR, "Failed to retrieve CPU index.");
+rc = ERROR_FAIL;
+goto out;
+}
+
+idx = libxl__json_object_get_integer(o);
+libxl_bitmap_set(map, idx);
+}
+
+rc = 0;
+out:
+GC_FREE;
+return rc;
+}
+
+int libxl__qmp_query_cpus(libxl__gc *gc, int domid, libxl_bitmap *map)
+{
+return qmp_run_command(gc, domid, "query-cpus", NULL,
+ query_cpus_callback, map);
+}
+
int libxl__qmp_nbd_server_start(libxl__gc *gc, int domid,
const char *host, const char *port)
{
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v4 2/5] libxl: libxl_domain_need_memory shouldn't modify b_info
This function is used to return the memory needed for a guest. It's not
in a position to modify the b_info passed in (note the _setdefault
function).
Constify the passed in b_info, use a copy to do the calculation. Mark
the change in API in libxl.h.
Signed-off-by: Wei Liu
---
Cc: Ian Jackson
v4: constify b_info and use LIBXL_HAVE_NEED_MEMORY_CONST_B_INFO
v3: new
---
tools/libxl/libxl.c | 8 +++-
tools/libxl/libxl.h | 10 +-
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 0c34d6b..51d202f 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -5121,12 +5121,17 @@ int libxl_get_memory_target(libxl_ctx *ctx, uint32_t
domid,
return rc;
}
-int libxl_domain_need_memory(libxl_ctx *ctx, libxl_domain_build_info *b_info,
+int libxl_domain_need_memory(libxl_ctx *ctx,
+ const libxl_domain_build_info *b_info_in,
uint32_t *need_memkb)
{
GC_INIT(ctx);
+libxl_domain_build_info b_info[1];
int rc;
+libxl_domain_build_info_init(b_info);
+libxl_domain_build_info_copy(ctx, b_info, b_info_in);
+
rc = libxl__domain_build_info_setdefault(gc, b_info);
if (rc) goto out;
@@ -5149,6 +5154,7 @@ int libxl_domain_need_memory(libxl_ctx *ctx,
libxl_domain_build_info *b_info,
rc = 0;
out:
GC_FREE;
+libxl_domain_build_info_dispose(b_info);
return rc;
}
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index f2843fd..48a43ce 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -74,6 +74,13 @@
*/
#define LIBXL_HAVE_CONST_COPY_AND_LENGTH_FUNCTIONS 1
+/* LIBXL_HAVE_DOMAIN_NEED_MEMORY_CONST_B_INFO
+ *
+ * If this is defined, libxl_domain_need_memory no longer modifies
+ * the b_info paseed in.
+ */
+#define LIBXL_HAVE_DOMAIN_NEED_MEMORY_CONST_B_INFO 1
+
/* LIBXL_HAVE_VNUMA
*
* If this is defined the type libxl_vnode_info exists, and a
@@ -1383,7 +1390,8 @@ int libxl_get_memory_target(libxl_ctx *ctx, uint32_t
domid, uint32_t *out_target
* existing programs which use them in roughly the same way as libxl.
*/
/* how much free memory in the system a domain needs to be built */
-int libxl_domain_need_memory(libxl_ctx *ctx, libxl_domain_build_info *b_info,
+int libxl_domain_need_memory(libxl_ctx *ctx,
+ const libxl_domain_build_info *b_info_in,
uint32_t *need_memkb);
/* how much free memory is available in the system */
int libxl_get_free_memory(libxl_ctx *ctx, uint32_t *memkb);
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v4 0/5] libxl: update available vcpus map in retrieved configuration
See individual patch for detailed changelog. Wei Liu (5): libxl: constify copy and length calculation functions libxl: libxl_domain_need_memory shouldn't modify b_info libxl: introduce libxl__qmp_query_cpus libxl: update vcpus bitmap in retrieved guest config libxl: only issue cpu-add call to QEMU for not present CPU tools/libxl/gentypes.py | 4 +- tools/libxl/libxl.c | 147 ++- tools/libxl/libxl.h | 33 +++--- tools/libxl/libxl_cpuid.c| 4 +- tools/libxl/libxl_genid.c| 2 +- tools/libxl/libxl_internal.h | 5 +- tools/libxl/libxl_qmp.c | 38 +++ tools/libxl/libxl_utils.c| 2 +- tools/libxl/libxl_utils.h| 2 +- 9 files changed, 204 insertions(+), 33 deletions(-) -- 2.1.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v1 02/20] acpi/hvmloader: Move acpi_info initialization out of ACPI code
Ian Jackson writes ("Re: [Xen-devel] [PATCH v1 02/20] acpi/hvmloader: Move
acpi_info initialization out of ACPI code"):
> Boris Ostrovsky writes ("Re: [Xen-devel] [PATCH v1 02/20] acpi/hvmloader:
> Move acpi_info initialization out of ACPI code"):
> > But do we need to look at who touched the file or who is explicitly
> > listed as copyright holder (in files' headers)?
>
> You need to look at the history.
>
> Sadly, we don't have any means of keeping the copyright headers up to
> date with lists of contributors.
Wait a moment, Lars points out that you are moving only certain bits
of these files into libxl.
Only the authorship of the moved parts is relevant.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v1 02/20] acpi/hvmloader: Move acpi_info initialization out of ACPI code
Boris Ostrovsky writes ("Re: [Xen-devel] [PATCH v1 02/20] acpi/hvmloader: Move
acpi_info initialization out of ACPI code"):
> But do we need to look at who touched the file or who is explicitly
> listed as copyright holder (in files' headers)?
You need to look at the history.
Sadly, we don't have any means of keeping the copyright headers up to
date with lists of contributors.
> So I did (because I thought only Signed-off might important, is it?)
No. In general I think From: is probably more relevant but I would
include both From: and S-o-b:.
If there are edge cases that crop up only once or twice they could be
excluded.
> ostr@workbase> git log . | grep Signed | sed -e 's/.*@/ /g' | sort | uniq
> citrix.com
> citrix.com>
> debian.org>
> eu.citrix.com>
> intel.com>
> jp.fujitsu.com>
> net-space.pl>
> novell.com>
> oracle.com>
> redhat.com>
> sun.com>
> suse.com>
> us.ibm.com>
> verge.net.au>
> xen.org>
> xensource.com>
That's a useful list.
Thanks,
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv1] xen/evtchn: add IOCTL_EVTCHN_RESTRICT
On 07/11/2016 10:57 AM, David Vrabel wrote:
> diff --git a/include/uapi/xen/evtchn.h b/include/uapi/xen/evtchn.h
> index 14e833ee4..f057b53 100644
> --- a/include/uapi/xen/evtchn.h
> +++ b/include/uapi/xen/evtchn.h
> @@ -85,4 +85,19 @@ struct ioctl_evtchn_notify {
> #define IOCTL_EVTCHN_RESET \
> _IOC(_IOC_NONE, 'E', 5, 0)
>
> +/*
> + * Restrict this file descriptor so that it can only be used to bind
> + * new interdomain events from one domain.
> + *
> + * Once a file descriptor has been restricted it cannot be
> + * de-restricted, and must be closed and re-opened. Event channels
> + * which were bound before restricting remain bound afterwards, and
> + * can be notified as usual.
> + */
> +#define IOCTL_EVTCHN_RESTRICT_DOMID \
> + _IOC(_IOC_NONE, 'E', 100, sizeof(struct ioctl_evtchn_restrict_domid))
Is there a reason why you picked 100 and not 6?
-boris
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 02/16] x86: fix: make atomic_read() param const
On 09/07/16 05:12, Corneliu ZUZU wrote:
> This wouldn't let me make a param of a function that used atomic_read() const.
>
> Signed-off-by: Corneliu ZUZU
This is a good improvement, but you must make an identical adjustment to
the arm code, otherwise you will end up with subtle build failures.
If you are really feeling up to it, having a common xen/atomic.h with
typedef struct { int counter; } atomic_t;
#define ATOMIC_INIT(i) { (i) }
and some prototypes such as:
static inline int atomic_read(const atomic_t *v);
would be great, but this looks like it has the possibility to turn into
a rats nest. If it does, then just doubling up this code for arm is ok.
~Andrew
> ---
> xen/include/asm-x86/atomic.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/xen/include/asm-x86/atomic.h b/xen/include/asm-x86/atomic.h
> index d246b70..0b250c8 100644
> --- a/xen/include/asm-x86/atomic.h
> +++ b/xen/include/asm-x86/atomic.h
> @@ -94,7 +94,7 @@ typedef struct { int counter; } atomic_t;
> *
> * Atomically reads the value of @v.
> */
> -static inline int atomic_read(atomic_t *v)
> +static inline int atomic_read(const atomic_t *v)
> {
> return read_atomic(&v->counter);
> }
> @@ -105,7 +105,7 @@ static inline int atomic_read(atomic_t *v)
> *
> * Non-atomically reads the value of @v
> */
> -static inline int _atomic_read(atomic_t v)
> +static inline int _atomic_read(const atomic_t v)
> {
> return v.counter;
> }
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v1 02/20] acpi/hvmloader: Move acpi_info initialization out of ACPI code
On 07/11/2016 10:54 AM, Konrad Rzeszutek Wilk wrote: > On Mon, Jul 11, 2016 at 03:47:20PM +0100, Lars Kurth wrote: >>> On 11 Jul 2016, at 13:10, Wei Liu wrote: >>> >>> CC Lars >>> >>> More licence stuff. Do you have contact(s)? >> James (on vacation) or I can confirm on behalf of Citrix. >> I am assuming that Konrad can get the approval for Sun/Oracle >> As for Intel, we can ask Susie Li. It would have to be a Intel decision >> because many of the people on that list are not working for Intel anymore. >> >> We should probably double check the dates of Keir's contributions: getting >> his explicit ACK is most likely not an issue >> >> According to LinkedIn, Stefan Berger still works for IBM. >> And a quick google search for Tobias Geiger's e-mail address and 2016 shows >> that the e-mail address was last used a month ago. >> >>> It would be far easier to ask the copyright holders: >>> >>> Andrew Cooper >>> Anthony PERARD >>> David Vrabel >>> Dexuan Cui >>> Eddie Dong >>> Ian Campbell >>> John Levon >>> Keir Fraser >>> Keir Fraser >>> Liu, Jinsong >>> Paul Durrant >>> Qing He >>> Stefan Berger >>> Tim Deegan >>> Tobias Geiger >>> Xiaowei Yang >> How was this list established? I get a different set of names for both >> build.c >> http://xenbits.xen.org/gitweb/?p=xen.git;a=history;f=tools/firmware/hvmloader/acpi/build.c >> acpi2_0.h >> http://xenbits.xen.org/gitweb/?p=xen.git;a=history;f=tools/firmware/hvmloader/acpi/acpi2_0.h >> >> Or did I miss anything? > I did: > > git log tools/firmware/hvmloader/acpi/acpi2_0.h | grep \@ | sed s/.*:// | > sort | uniq > And removed the duplicate entries. > > But hadn't done the build.c which has: > > [konrad@char xen]$ git log tools/firmware/hvmloader/acpi/build.c | grep \@ > | sed s/.*:// | sort | uniq > Adnan Misherfi > Andrew Cooper > Anthony PERARD > Atsushi SAKAI > Christoph Egger > Dan Magenheimer > David Vrabel David Vrabel > Dexuan Cui > Eddie Dong > Frediano Ziglio > Ian Campbell > Jan Beulich > kaf24@localhost.localdomain > Kamala Narasimhan > Keir Fraser > Keir Fraser > Keir Fraser > Keir Fraser > kfraser@localhost.localdomain > Konrad Rzeszutek Wilk > Liu, Jinsong > Paolo Bonzini > Paul Durrant > Qing He > Ross Philipson > Samuel Thibault > Trolle Selander > Xiaowei Yang > > > Thought > http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=883236e49a86a0174c6df61cac995ebf16d72b35 > shows that the file was moved, and I hadn't taken that into account. > > So would need to look even further back under a different directory. But do we need to look at who touched the file or who is explicitly listed as copyright holder (in files' headers)? So I did (because I thought only Signed-off might important, is it?) ostr@workbase> git log . | grep Signed | sed -e 's/.*@/ /g' | sort | uniq citrix.com citrix.com> debian.org> eu.citrix.com> intel.com> jp.fujitsu.com> net-space.pl> novell.com> oracle.com> redhat.com> sun.com> suse.com> us.ibm.com> verge.net.au> xen.org> xensource.com> -boris ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 4/5] libxl: update vcpus bitmap in retrieved guest config
On Mon, Jul 11, 2016 at 12:31:43PM +0100, Wei Liu wrote: [...] > > > > > > +/* If the user did not explicitly specify a device model > > > + * version, we need to use the same logic used during domain > > > + * creation to derive the device model version. > > > + */ > > > +version = d_config->b_info.device_model_version; > > > +if (version == LIBXL_DEVICE_MODEL_VERSION_UNKNOWN) > > > +version = libxl__get_device_model_version(gc, > > > &d_config->b_info); > > > > I think this is rather unfortunate. Won't changing the default device > > model while the domain is running will cause this function to give > > wrong answers ? > > I think saving the device model into the template should work. No need > to derive it during runtime. > Actually this information is already available via libxl__device_model_version_running. Wei. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCHv1] xen/evtchn: add IOCTL_EVTCHN_RESTRICT
IOCTL_EVTCHN_RESTRICT limits the file descriptor to being able to bind
to interdomain event channels from a specific domain. Event channels
that are already bound continue to work for sending and receiving
notifications.
This is useful as part of deprivileging a user space PV backend or
device model (QEMU). e.g., Once the device model as bound to the
ioreq server event channels it can restrict the file handle so an
exploited DM cannot use it to create or bind to arbitrary event
channels.
Signed-off-by: David Vrabel
---
drivers/xen/evtchn.c | 40
include/uapi/xen/evtchn.h | 15 +++
2 files changed, 55 insertions(+)
diff --git a/drivers/xen/evtchn.c b/drivers/xen/evtchn.c
index f4edd6d..7efd1cb 100644
--- a/drivers/xen/evtchn.c
+++ b/drivers/xen/evtchn.c
@@ -73,8 +73,12 @@ struct per_user_data {
wait_queue_head_t evtchn_wait;
struct fasync_struct *evtchn_async_queue;
const char *name;
+
+ domid_t restrict_domid;
};
+#define UNRESTRICTED_DOMID ((domid_t)-1)
+
struct user_evtchn {
struct rb_node node;
struct per_user_data *user;
@@ -443,6 +447,10 @@ static long evtchn_ioctl(struct file *file,
struct ioctl_evtchn_bind_virq bind;
struct evtchn_bind_virq bind_virq;
+ rc = -EACCES;
+ if (u->restrict_domid != UNRESTRICTED_DOMID)
+ break;
+
rc = -EFAULT;
if (copy_from_user(&bind, uarg, sizeof(bind)))
break;
@@ -468,6 +476,11 @@ static long evtchn_ioctl(struct file *file,
if (copy_from_user(&bind, uarg, sizeof(bind)))
break;
+ rc = -EACCES;
+ if (u->restrict_domid != UNRESTRICTED_DOMID &&
+ u->restrict_domid != bind.remote_domain)
+ break;
+
bind_interdomain.remote_dom = bind.remote_domain;
bind_interdomain.remote_port = bind.remote_port;
rc = HYPERVISOR_event_channel_op(EVTCHNOP_bind_interdomain,
@@ -485,6 +498,10 @@ static long evtchn_ioctl(struct file *file,
struct ioctl_evtchn_bind_unbound_port bind;
struct evtchn_alloc_unbound alloc_unbound;
+ rc = -EACCES;
+ if (u->restrict_domid != UNRESTRICTED_DOMID)
+ break;
+
rc = -EFAULT;
if (copy_from_user(&bind, uarg, sizeof(bind)))
break;
@@ -553,6 +570,27 @@ static long evtchn_ioctl(struct file *file,
break;
}
+ case IOCTL_EVTCHN_RESTRICT_DOMID: {
+ struct ioctl_evtchn_restrict_domid ierd;
+
+ rc = -EACCES;
+ if (u->restrict_domid != UNRESTRICTED_DOMID)
+ break;
+
+ rc = -EFAULT;
+ if (copy_from_user(&ierd, uarg, sizeof(ierd)))
+ break;
+
+ rc = -EINVAL;
+ if (ierd.domid == 0 || ierd.domid >= DOMID_FIRST_RESERVED)
+ break;
+
+ u->restrict_domid = ierd.domid;
+ rc = 0;
+
+ break;
+ }
+
default:
rc = -ENOSYS;
break;
@@ -601,6 +639,8 @@ static int evtchn_open(struct inode *inode, struct file
*filp)
mutex_init(&u->ring_cons_mutex);
spin_lock_init(&u->ring_prod_lock);
+ u->restrict_domid = UNRESTRICTED_DOMID;
+
filp->private_data = u;
return nonseekable_open(inode, filp);
diff --git a/include/uapi/xen/evtchn.h b/include/uapi/xen/evtchn.h
index 14e833ee4..f057b53 100644
--- a/include/uapi/xen/evtchn.h
+++ b/include/uapi/xen/evtchn.h
@@ -85,4 +85,19 @@ struct ioctl_evtchn_notify {
#define IOCTL_EVTCHN_RESET \
_IOC(_IOC_NONE, 'E', 5, 0)
+/*
+ * Restrict this file descriptor so that it can only be used to bind
+ * new interdomain events from one domain.
+ *
+ * Once a file descriptor has been restricted it cannot be
+ * de-restricted, and must be closed and re-opened. Event channels
+ * which were bound before restricting remain bound afterwards, and
+ * can be notified as usual.
+ */
+#define IOCTL_EVTCHN_RESTRICT_DOMID\
+ _IOC(_IOC_NONE, 'E', 100, sizeof(struct ioctl_evtchn_restrict_domid))
+struct ioctl_evtchn_restrict_domid {
+ domid_t domid;
+};
+
#endif /* __LINUX_PUBLIC_EVTCHN_H__ */
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v1 02/20] acpi/hvmloader: Move acpi_info initialization out of ACPI code
On Mon, Jul 11, 2016 at 03:47:20PM +0100, Lars Kurth wrote: > > > On 11 Jul 2016, at 13:10, Wei Liu wrote: > > > > CC Lars > > > > More licence stuff. Do you have contact(s)? > > James (on vacation) or I can confirm on behalf of Citrix. > I am assuming that Konrad can get the approval for Sun/Oracle > As for Intel, we can ask Susie Li. It would have to be a Intel decision > because many of the people on that list are not working for Intel anymore. > > We should probably double check the dates of Keir's contributions: getting > his explicit ACK is most likely not an issue > > According to LinkedIn, Stefan Berger still works for IBM. > And a quick google search for Tobias Geiger's e-mail address and 2016 shows > that the e-mail address was last used a month ago. > > > It would be far easier to ask the copyright holders: > > > > Andrew Cooper > > Anthony PERARD > > David Vrabel > > Dexuan Cui > > Eddie Dong > > Ian Campbell > > John Levon > > Keir Fraser > > Keir Fraser > > Liu, Jinsong > > Paul Durrant > > Qing He > > Stefan Berger > > Tim Deegan > > Tobias Geiger > > Xiaowei Yang > > How was this list established? I get a different set of names for both > build.c > http://xenbits.xen.org/gitweb/?p=xen.git;a=history;f=tools/firmware/hvmloader/acpi/build.c > acpi2_0.h > http://xenbits.xen.org/gitweb/?p=xen.git;a=history;f=tools/firmware/hvmloader/acpi/acpi2_0.h > > Or did I miss anything? I did: git log tools/firmware/hvmloader/acpi/acpi2_0.h | grep \@ | sed s/.*:// | sort | uniq And removed the duplicate entries. But hadn't done the build.c which has: [konrad@char xen]$ git log tools/firmware/hvmloader/acpi/build.c | grep \@ | sed s/.*:// | sort | uniq Adnan Misherfi Andrew Cooper Anthony PERARD Atsushi SAKAI Christoph Egger Dan Magenheimer David Vrabel Dexuan Cui Eddie Dong Frediano Ziglio Ian Campbell Jan Beulich kaf24@localhost.localdomain Kamala Narasimhan Keir Fraser Keir Fraser Keir Fraser Keir Fraser kfraser@localhost.localdomain Konrad Rzeszutek Wilk Liu, Jinsong Paolo Bonzini Paul Durrant Qing He Ross Philipson Samuel Thibault Trolle Selander Xiaowei Yang Thought http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=883236e49a86a0174c6df61cac995ebf16d72b35 shows that the file was moved, and I hadn't taken that into account. So would need to look even further back under a different directory. > > Regards > Lars ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [DRAFT 1] XenSock protocol design document
On 07/08/2016 12:23 PM, Stefano Stabellini wrote:
> Hi all,
>
Hey!
[...]
>
> ## Design
>
> ### Xenstore
>
> The frontend and the backend connect to each other exchanging information via
> xenstore. The toolstack creates front and back nodes with state
> XenbusStateInitialising. There can only be one XenSock frontend per domain.
>
> Frontend XenBus Nodes
>
> port
> Values:
>
> The identifier of the Xen event channel used to signal activity
> in the ring buffer.
>
> ring-ref
> Values:
>
> The Xen grant reference granting permission for the backend to map
> the sole page in a single page sized ring buffer.
Would it make sense to export minimum, default and maximum size of the socket
over
xenstore entries? It normally follows a convention depending on the type of
socket
(and OS) you have, or then through settables on socket options.
> ### Commands Ring
>
> The shared ring is used by the frontend to forward socket API calls to the
> backend. I'll refer to this ring as **commands ring** to distinguish it from
> other rings which will be created later in the lifecycle of the protocol (data
> rings). The ring format is defined using the familiar `DEFINE_RING_TYPES`
> macro
> (`xen/include/public/io/ring.h`). Frontend requests are allocated on the ring
> using the `RING_GET_REQUEST` macro.
>
> The format is defined as follows:
>
> #define XENSOCK_DATARING_ORDER 6
> #define XENSOCK_DATARING_PAGES (1 << XENSOCK_DATARING_ORDER)
> #define XENSOCK_DATARING_SIZE (XENSOCK_DATARING_PAGES << PAGE_SHIFT)
>
> #define XENSOCK_CONNECT0
> #define XENSOCK_RELEASE3
> #define XENSOCK_BIND 4
> #define XENSOCK_LISTEN 5
> #define XENSOCK_ACCEPT 6
> #define XENSOCK_POLL 7
>
> struct xen_xensock_request {
> uint32_t id; /* private to guest, echoed in response */
> uint32_t cmd;/* command to execute */
> uint64_t sockid; /* id of the socket */
> union {
> struct xen_xensock_connect {
> uint8_t addr[28];
> uint32_t len;
> uint32_t flags;
> grant_ref_t ref[XENSOCK_DATARING_PAGES];
> uint32_t evtchn;
> } connect;
> struct xen_xensock_bind {
> uint8_t addr[28]; /* ipv6 ready */
> uint32_t len;
> } bind;
> struct xen_xensock_accept {
> uint64_t sockid;
> grant_ref_t ref[XENSOCK_DATARING_PAGES];
> uint32_t evtchn;
> } accept;
> } u;
> };
>
> The first three fields are common for every command. Their binary layout
> is:
>
> 0 4 8 12 16
> +---+---+---+---+
> | id | cmd | sockid|
> +---+---+---+---+
>
> - **id** is generated by the frontend and identifies one specific request
> - **cmd** is the command requested by the frontend:
> - `XENSOCK_CONNECT`: 0
> - `XENSOCK_RELEASE`: 3
> - `XENSOCK_BIND`:4
> - `XENSOCK_LISTEN`: 5
> - `XENSOCK_ACCEPT`: 6
> - `XENSOCK_POLL`:7
> - **sockid** is generated by the frontend and identifies the socket to
> connect,
> bind, etc. A new sockid is required on `XENSOCK_CONNECT` and `XENSOCK_BIND`
> commands. A new sockid is also required on `XENSOCK_ACCEPT`, for the new
> socket.
>
Interesting - Have you consider setsockopt and getsockopt to be part of this?
There
are some common options (as in POSIX defined) and then some more exotic flavors
Linux
or FreeBSD specific. Say SO_REUSEPORT used on nginx that is good for load
balancing
across a set of workers or Linux SO_BUSY_POLL for low latency sockets. Though
not
sure how sensible it is to start exposing all of these socket options but to
limit to
a specific subset? Or maybe doesn't make sense for your case - see further
suggestion
regarding data ring part.
> All three fields are echoed back by the backend.
>
> As for the other Xen ring based protocols, after writing a request to the
> ring,
> the frontend calls `RING_PUSH_REQUESTS_AND_CHECK_NOTIFY` and issues an event
> channel notification when a notification is required.
>
> Backend responses are allocated on the ring using the `RING_GET_RESPONSE`
> macro.
> The format is the following:
>
> struct xen_xensock_response {
> uint32_t id;
> uint32_t cmd;
> uint64_t sockid;
> int32_t ret;
> };
>
> 0 4 8 12 16 20
> +---+---+---+---+---+
> | id | cmd | sockid| ret |
> +---+---+---+---+---+
>
> - **id**: echoed back from request
> - **cmd**: echoed back from request
> - **sockid**: echoed back from request
> - **ret**: return value, identifies success or failure
>
Are these fields taken from
Re: [Xen-devel] [PATCH v1 02/20] acpi/hvmloader: Move acpi_info initialization out of ACPI code
> On 11 Jul 2016, at 13:10, Wei Liu wrote: > > CC Lars > > More licence stuff. Do you have contact(s)? James (on vacation) or I can confirm on behalf of Citrix. I am assuming that Konrad can get the approval for Sun/Oracle As for Intel, we can ask Susie Li. It would have to be a Intel decision because many of the people on that list are not working for Intel anymore. We should probably double check the dates of Keir's contributions: getting his explicit ACK is most likely not an issue According to LinkedIn, Stefan Berger still works for IBM. And a quick google search for Tobias Geiger's e-mail address and 2016 shows that the e-mail address was last used a month ago. > It would be far easier to ask the copyright holders: > > Andrew Cooper > Anthony PERARD > David Vrabel > Dexuan Cui > Eddie Dong > Ian Campbell > John Levon > Keir Fraser > Keir Fraser > Liu, Jinsong > Paul Durrant > Qing He > Stefan Berger > Tim Deegan > Tobias Geiger > Xiaowei Yang How was this list established? I get a different set of names for both build.c http://xenbits.xen.org/gitweb/?p=xen.git;a=history;f=tools/firmware/hvmloader/acpi/build.c acpi2_0.h http://xenbits.xen.org/gitweb/?p=xen.git;a=history;f=tools/firmware/hvmloader/acpi/acpi2_0.h Or did I miss anything? Regards Lars ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v1 20/20] libxl/acpi: Build ACPI tables for HVMlite guests
On 07/11/2016 09:41 AM, Wei Liu wrote:
> On Mon, Jul 11, 2016 at 09:33:21AM -0400, Boris Ostrovsky wrote:
>> On 07/11/2016 06:47 AM, Wei Liu wrote:
>>> On Fri, Jul 08, 2016 at 01:20:46PM -0400, Boris Ostrovsky wrote:
On 07/08/2016 12:07 PM, Wei Liu wrote:
> On Fri, Jul 08, 2016 at 10:48:52AM -0400, Boris Ostrovsky wrote:
>> On 07/08/2016 06:55 AM, Wei Liu wrote:
+
+/* Map page that will hold RSDP */
+extent = RSDP_ADDRESS >> ctxt.page_shift;
+rc = populate_acpi_pages(dom, &extent, 1, &ctxt);
+if (rc) {
+LOG(ERROR, "%s: populate_acpi_pages for rsdp failed with %d",
+__FUNCTION__, rc);
+goto out;
+}
+config.rsdp = (unsigned long)xc_map_foreign_range(xch, domid,
+ ctxt.page_size,
+ PROT_READ |
PROT_WRITE,
+ RSDP_ADDRESS >>
ctxt.page_shift);
>>> I think with Anthony's on-going work you should be more flexible for all
>>> you tables.
>> Not sure I understand what you mean here. You want the address
>> (RSDP_ADDRESS) to be a variable as opposed to a macro?
>>
> I'm still trying to wrap my head around the possible interaction between
> Anthony's work and your work.
>
> Anthony's work allows dynamically loading of firmware blobs. If you use
> a fixed address, theoretically it can clash with firmware blobs among
> other things libxc can load. The high address is a safe bet so that
> probably won't happen in practice.
>
> Anthony's work allows loading arbitrary blobs actually. Can you take
> advantage of that mechanism as well? That is, to specify all your tables
> as modules and let libxc handle actually loading them into guest
> memory.
>
> Does this make sense?
>
> Also CC Anthony here.
My understanding of Anthony's series is that its goal was to provide an
interface to pass DSDT (and maybe some other tables) from the toolstack
to hvmloader.
Here we don't have hvmloader, we are loading the tables directly into
guest's memory.
>>> Do you use the same hvm_start_info structure? I don't think that
>>> structure is restricted to hvmloader.
>>
>> Yes, we do. However, in PVH(v2) case it will be seen next by the guest
>> who will expect the tables to already be in memory. I.e. there is no
>> intermediate Xen component, such as hvmloader, who can load the blobs.
>>
> Maybe you misunderstood. Anthony's work will load the blobs into guest
> memory -- all fields in hvm_start_info points to guest physical address
> IIRC. Hvmloader might want to relocate them, but that's a different
> matter.
What's the status on Anthony's series? I can rebase on top of his tree
(I might indeed be able to reuse some of his code) but if this is way
off the dependencies become problematic (because Shannon's series would
also be delayed).
>
>> Having said that, I wonder whether we (both x86 and ARM) could use
>> Anthony's xc_dom_image.full_acpi_module instead
(no full_acpi_module anymore, I was looking at an earlier series
version. I guess it's system_firmware_module now)
>> of acpitables_blob that
>> Shannon's series added. (even if we can't, I think
>> xc_hvm_firmware_module is the right datastructure to store the blob
>> since it has both toolstack's virtual and guest's physical addresses).
>>
> Yes, that's along the line I'm thinking about.
So I am confused about xc_hvm_firmware_mode: is guest_addr_out meant to
be guest's physical or virtual?
One one hand it looks like virtual:
in
https://lists.xenproject.org/archives/html/xen-devel/2016-06/msg02901.html
+module->guest_addr_out = seg.vstart;
but then in
https://lists.xenproject.org/archives/html/xen-devel/2016-06/msg02902.html
+modlist[index].paddr = module->guest_addr_out;
-boris
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [BUG] kernel BUG at drivers/block/xen-blkfront.c:1711
On 11.07.2016 13:37, George Dunlap wrote: On Mon, Jul 11, 2016 at 9:50 AM, Evgenii Shatokhin wrote: On 06.06.2016 11:42, Dario Faggioli wrote: Just Cc-ing some Linux, block, and Xen on CentOS people... Ping. Any suggestions how to debug this or what might cause the problem? Obviously, we cannot control Xen on the Amazon's servers. But perhaps there is something we can do at the kernel's side, is it? I think part of the problem is that your report has confused people so that everyone cc'd thinks it's someone else's problem. :-) To wit.. One of our users gets kernel panics from time to time when he tries to use his Amazon EC2 instance with CentOS7 x64 in it [1]. Kernel panic happens within minutes from the moment the instance starts. The problem does not show up every time, however. The user first observed the problem with a custom kernel, but it was found later that the stock kernel 3.10.0-327.18.2.el7.x86_64 from CentOS7 was affected as well. ...by mentioning the exact CentOS kernel version, but not the version of the "custom kernel" you used, I suspect the people familiar with netfront filtered this out as something to be taken care of by the CentOS / RHEL system. The custom kernel is based on the same RHEL's kernel 3.10.0-327.18.2.el7.x86_64 as the one from CentOS 7. We did not change Xen-related parts there. If you can reproduce this with a relatively recent stock kernel, please post the kernel version and the debug information. If you can't, then it's likely to be an issue that RH needs to take care of by backporting whatever change fixed the issue. Thanks for the suggestion. Yes, if the patch Bob Liu has proposed does not help, I will try to reproduce the problem on a recent mainline kernel. The difficult part is that the problem is rather hard to reproduce, but, well, it is another story. Thanks, -George . Regards, Evgenii ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [BUG] kernel BUG at drivers/block/xen-blkfront.c:1711
On 11.07.2016 15:04, Bob Liu wrote:
On 07/11/2016 04:50 PM, Evgenii Shatokhin wrote:
On 06.06.2016 11:42, Dario Faggioli wrote:
Just Cc-ing some Linux, block, and Xen on CentOS people...
Ping.
Any suggestions how to debug this or what might cause the problem?
Obviously, we cannot control Xen on the Amazon's servers. But perhaps there is
something we can do at the kernel's side, is it?
On Mon, 2016-06-06 at 11:24 +0300, Evgenii Shatokhin wrote:
(Resending this bug report because the message I sent last week did
not
make it to the mailing list somehow.)
Hi,
One of our users gets kernel panics from time to time when he tries
to
use his Amazon EC2 instance with CentOS7 x64 in it [1]. Kernel panic
happens within minutes from the moment the instance starts. The
problem
does not show up every time, however.
The user first observed the problem with a custom kernel, but it was
found later that the stock kernel 3.10.0-327.18.2.el7.x86_64 from
CentOS7 was affected as well.
Please try this patch:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=7b0767502b5db11cb1f0daef2d01f6d71b1192dc
Thanks! I have rebuilt our kernel (based on RHEL's
3.10.0-327.18.2.el7.x86_64) with that patch added and asked that user to
try it. Let us see if it helps.
Regards,
Evgenii
Regards,
Bob
The part of the system log he was able to retrieve is attached. Here
is
the bug info, for convenience:
[2.246912] kernel BUG at drivers/block/xen-blkfront.c:1711!
[2.246912] invalid opcode: [#1] SMP
[2.246912] Modules linked in: ata_generic pata_acpi
crct10dif_pclmul
crct10dif_common crc32_pclmul crc32c_intel ghash_clmulni_intel
xen_netfront xen_blkfront(+) aesni_intel lrw ata_piix gf128mul
glue_helper ablk_helper cryptd libata serio_raw floppy sunrpc
dm_mirror
dm_region_hash dm_log dm_mod scsi_transport_iscsi
[2.246912] CPU: 1 PID: 50 Comm: xenwatch Not tainted
3.10.0-327.18.2.el7.x86_64 #1
[2.246912] Hardware name: Xen HVM domU, BIOS 4.2.amazon
12/07/2015
[2.246912] task: 8800e9fcb980 ti: 8800e98bc000 task.ti:
8800e98bc000
[2.246912] RIP: 0010:[] []
blkfront_setup_indirect+0x41f/0x430 [xen_blkfront]
[2.246912] RSP: 0018:8800e98bfcd0 EFLAGS: 00010283
[2.246912] RAX: 8800353e15c0 RBX: 8800e98c52c8 RCX:
0020
[2.246912] RDX: 8800353e15b0 RSI: 8800e98c52b8 RDI:
8800353e15d0
[2.246912] RBP: 8800e98bfd20 R08: 8800353e15b0 R09:
8800eb403c00
[2.246912] R10: a0155532 R11: ffe8 R12:
8800e98c4000
[2.246912] R13: 8800e98c52b8 R14: 0020 R15:
8800353e15c0
[2.246912] FS: () GS:8800efc2()
knlGS:
[2.246912] CS: 0010 DS: ES: CR0: 80050033
[2.246912] CR2: 7f1b615ef000 CR3: e2b44000 CR4:
001406e0
[2.246912] DR0: DR1: DR2:
[2.246912] DR3: DR6: 0ff0 DR7:
0400
[2.246912] Stack:
[2.246912] 0020 0001 0020a0157217
0100e98bfdbc
[2.246912] 27efa3ef 8800e98bfdbc 8800e98ce000
8800e98c4000
[2.246912] 8800e98ce040 0001 8800e98bfe08
a0155d4c
[2.246912] Call Trace:
[2.246912] [] blkback_changed+0x4ec/0xfc8
[xen_blkfront]
[2.246912] [] ? xenbus_gather+0x170/0x190
[2.246912] [] ? __slab_free+0x10e/0x277
[2.246912] []
xenbus_otherend_changed+0xad/0x110
[2.246912] [] ? xenwatch_thread+0x77/0x180
[2.246912] [] backend_changed+0x13/0x20
[2.246912] [] xenwatch_thread+0x66/0x180
[2.246912] [] ? wake_up_atomic_t+0x30/0x30
[2.246912] [] ?
unregister_xenbus_watch+0x1f0/0x1f0
[2.246912] [] kthread+0xcf/0xe0
[2.246912] [] ?
kthread_create_on_node+0x140/0x140
[2.246912] [] ret_from_fork+0x58/0x90
[2.246912] [] ?
kthread_create_on_node+0x140/0x140
[2.246912] Code: e1 48 85 c0 75 ce 49 8d 84 24 40 01 00 00 48 89
45
b8 e9 91 fd ff ff 4c 89 ff e8 8d ae 06 e1 e9 f2 fc ff ff 31 c0 e9 2e
fe
ff ff <0f> 0b e8 9a 57 f2 e0 0f 0b 0f 1f 84 00 00 00 00 00 0f 1f 44
00
[2.246912] RIP []
blkfront_setup_indirect+0x41f/0x430 [xen_blkfront]
[2.246912] RSP
[2.491574] ---[ end trace 8a9b992812627c71 ]---
[2.495618] Kernel panic - not syncing: Fatal exception
Xen version 4.2.
EC2 instance type: c3.large with EBS magnetic storage, if that
matters.
Here is the code where the BUG_ON triggers (drivers/block/xen-
blkfront.c):
if (!info->feature_persistent && info->max_indirect_segments) {
/*
* We are using indirect descriptors but not persistent
* grants, we need to allocate a set of pages that can be
* used for mapping indirect grefs
*/
int num = INDIRECT_GR
Re: [Xen-devel] [PATCH v1 20/20] libxl/acpi: Build ACPI tables for HVMlite guests
On Mon, Jul 11, 2016 at 09:33:21AM -0400, Boris Ostrovsky wrote:
> On 07/11/2016 06:47 AM, Wei Liu wrote:
> > On Fri, Jul 08, 2016 at 01:20:46PM -0400, Boris Ostrovsky wrote:
> >> On 07/08/2016 12:07 PM, Wei Liu wrote:
> >>> On Fri, Jul 08, 2016 at 10:48:52AM -0400, Boris Ostrovsky wrote:
> On 07/08/2016 06:55 AM, Wei Liu wrote:
> >> +
> >> +/* Map page that will hold RSDP */
> >> +extent = RSDP_ADDRESS >> ctxt.page_shift;
> >> +rc = populate_acpi_pages(dom, &extent, 1, &ctxt);
> >> +if (rc) {
> >> +LOG(ERROR, "%s: populate_acpi_pages for rsdp failed with %d",
> >> +__FUNCTION__, rc);
> >> +goto out;
> >> +}
> >> +config.rsdp = (unsigned long)xc_map_foreign_range(xch, domid,
> >> + ctxt.page_size,
> >> + PROT_READ |
> >> PROT_WRITE,
> >> + RSDP_ADDRESS >>
> >> ctxt.page_shift);
> > I think with Anthony's on-going work you should be more flexible for all
> > you tables.
If the tool stack already knows where it can (or should) load the rsdp,
there is not much reason to be flexible.
> Not sure I understand what you mean here. You want the address
> (RSDP_ADDRESS) to be a variable as opposed to a macro?
>
> >>> I'm still trying to wrap my head around the possible interaction between
> >>> Anthony's work and your work.
> >>>
> >>> Anthony's work allows dynamically loading of firmware blobs. If you use
> >>> a fixed address, theoretically it can clash with firmware blobs among
> >>> other things libxc can load. The high address is a safe bet so that
> >>> probably won't happen in practice.
> >>>
> >>> Anthony's work allows loading arbitrary blobs actually. Can you take
> >>> advantage of that mechanism as well? That is, to specify all your tables
> >>> as modules and let libxc handle actually loading them into guest
> >>> memory.
> >>>
> >>> Does this make sense?
> >>>
> >>> Also CC Anthony here.
> >>
> >> My understanding of Anthony's series is that its goal was to provide an
> >> interface to pass DSDT (and maybe some other tables) from the toolstack
> >> to hvmloader.
Not anymore. The only new functionality provided by the patch series is
to load the BIOS (or OVMF) from the filesystem (instead of having this
blob embedded into hvmloader).
It does also change the way an extra acpi tables or a smbios is loaded
into guest memory for consumption by hvmloader. But that just make the
libxc code a bit cleaner.
> >> Here we don't have hvmloader, we are loading the tables directly into
> >> guest's memory.
> >>
> > Do you use the same hvm_start_info structure? I don't think that
> > structure is restricted to hvmloader.
>
>
> Yes, we do. However, in PVH(v2) case it will be seen next by the guest
> who will expect the tables to already be in memory. I.e. there is no
> intermediate Xen component, such as hvmloader, who can load the blobs.
>
> Having said that, I wonder whether we (both x86 and ARM) could use
> Anthony's xc_dom_image.full_acpi_module instead of acpitables_blob that
I don't have full_acpi_module anymore in my patch series. But that does
not prevent you from introducing one.
> Shannon's series added. (even if we can't, I think
> xc_hvm_firmware_module is the right datastructure to store the blob
> since it has both toolstack's virtual and guest's physical addresses).
--
Anthony PERARD
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v1 20/20] libxl/acpi: Build ACPI tables for HVMlite guests
On 11/07/16 14:42, Wei Liu wrote: On Mon, Jul 11, 2016 at 02:39:05PM +0100, Julien Grall wrote: Yes, we do. However, in PVH(v2) case it will be seen next by the guest who will expect the tables to already be in memory. I.e. there is no intermediate Xen component, such as hvmloader, who can load the blobs. Having said that, I wonder whether we (both x86 and ARM) could use Anthony's xc_dom_image.full_acpi_module instead of acpitables_blob that Shannon's series added. (even if we can't, I think xc_hvm_firmware_module is the right datastructure to store the blob since it has both toolstack's virtual and guest's physical addresses). In this case, xc_hvm_firmware_module would need to be renamed as ARM guests are neither HVM nor PV. That's trivial. It's an internal structure that we can rename at will. FWIW, from the toolstack point of view, ARM guests is considered as PV guest. ... while at the same time utilises HVM param... Not complaining, just this makes me chuckle a bit. :-) ARM guests is a combination of HVM and PV features. I agree it is a bit a mess, but there is code in the hypervisor/toolstack/Linux which rely on the type of guests (e.g LIBXL_DOMAIN_TYPE_* in libxl) and not a set of available features. In the hypervisor, we are trying to move towards a set of features (i.e dropping is_pv_domain/is_hvm_domain in common code) as none suit ARM guests. I think it will benefit for both ARM and x86 to move available features rather than type. However this is requiring a lot of rework which cannot be done quickly. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v1 20/20] libxl/acpi: Build ACPI tables for HVMlite guests
On Mon, Jul 11, 2016 at 02:39:05PM +0100, Julien Grall wrote: > >Yes, we do. However, in PVH(v2) case it will be seen next by the guest > >who will expect the tables to already be in memory. I.e. there is no > >intermediate Xen component, such as hvmloader, who can load the blobs. > > > >Having said that, I wonder whether we (both x86 and ARM) could use > >Anthony's xc_dom_image.full_acpi_module instead of acpitables_blob that > >Shannon's series added. (even if we can't, I think > >xc_hvm_firmware_module is the right datastructure to store the blob > >since it has both toolstack's virtual and guest's physical addresses). > > In this case, xc_hvm_firmware_module would need to be renamed as ARM guests > are neither HVM nor PV. > That's trivial. It's an internal structure that we can rename at will. > FWIW, from the toolstack point of view, ARM guests is considered as PV > guest. ... while at the same time utilises HVM param... Not complaining, just this makes me chuckle a bit. :-) Wei. > > Regards, > > -- > Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v1 20/20] libxl/acpi: Build ACPI tables for HVMlite guests
On Mon, Jul 11, 2016 at 09:33:21AM -0400, Boris Ostrovsky wrote:
> On 07/11/2016 06:47 AM, Wei Liu wrote:
> > On Fri, Jul 08, 2016 at 01:20:46PM -0400, Boris Ostrovsky wrote:
> >> On 07/08/2016 12:07 PM, Wei Liu wrote:
> >>> On Fri, Jul 08, 2016 at 10:48:52AM -0400, Boris Ostrovsky wrote:
> On 07/08/2016 06:55 AM, Wei Liu wrote:
> >> +
> >> +/* Map page that will hold RSDP */
> >> +extent = RSDP_ADDRESS >> ctxt.page_shift;
> >> +rc = populate_acpi_pages(dom, &extent, 1, &ctxt);
> >> +if (rc) {
> >> +LOG(ERROR, "%s: populate_acpi_pages for rsdp failed with %d",
> >> +__FUNCTION__, rc);
> >> +goto out;
> >> +}
> >> +config.rsdp = (unsigned long)xc_map_foreign_range(xch, domid,
> >> + ctxt.page_size,
> >> + PROT_READ |
> >> PROT_WRITE,
> >> + RSDP_ADDRESS >>
> >> ctxt.page_shift);
> > I think with Anthony's on-going work you should be more flexible for all
> > you tables.
> Not sure I understand what you mean here. You want the address
> (RSDP_ADDRESS) to be a variable as opposed to a macro?
>
> >>> I'm still trying to wrap my head around the possible interaction between
> >>> Anthony's work and your work.
> >>>
> >>> Anthony's work allows dynamically loading of firmware blobs. If you use
> >>> a fixed address, theoretically it can clash with firmware blobs among
> >>> other things libxc can load. The high address is a safe bet so that
> >>> probably won't happen in practice.
> >>>
> >>> Anthony's work allows loading arbitrary blobs actually. Can you take
> >>> advantage of that mechanism as well? That is, to specify all your tables
> >>> as modules and let libxc handle actually loading them into guest
> >>> memory.
> >>>
> >>> Does this make sense?
> >>>
> >>> Also CC Anthony here.
> >>
> >> My understanding of Anthony's series is that its goal was to provide an
> >> interface to pass DSDT (and maybe some other tables) from the toolstack
> >> to hvmloader.
> >>
> >> Here we don't have hvmloader, we are loading the tables directly into
> >> guest's memory.
> >>
> > Do you use the same hvm_start_info structure? I don't think that
> > structure is restricted to hvmloader.
>
>
> Yes, we do. However, in PVH(v2) case it will be seen next by the guest
> who will expect the tables to already be in memory. I.e. there is no
> intermediate Xen component, such as hvmloader, who can load the blobs.
>
Maybe you misunderstood. Anthony's work will load the blobs into guest
memory -- all fields in hvm_start_info points to guest physical address
IIRC. Hvmloader might want to relocate them, but that's a different
matter.
> Having said that, I wonder whether we (both x86 and ARM) could use
> Anthony's xc_dom_image.full_acpi_module instead of acpitables_blob that
> Shannon's series added. (even if we can't, I think
> xc_hvm_firmware_module is the right datastructure to store the blob
> since it has both toolstack's virtual and guest's physical addresses).
>
Yes, that's along the line I'm thinking about.
Wei.
> -boris
>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v1 20/20] libxl/acpi: Build ACPI tables for HVMlite guests
On 11/07/16 14:33, Boris Ostrovsky wrote:
On 07/11/2016 06:47 AM, Wei Liu wrote:
On Fri, Jul 08, 2016 at 01:20:46PM -0400, Boris Ostrovsky wrote:
On 07/08/2016 12:07 PM, Wei Liu wrote:
On Fri, Jul 08, 2016 at 10:48:52AM -0400, Boris Ostrovsky wrote:
On 07/08/2016 06:55 AM, Wei Liu wrote:
+
+/* Map page that will hold RSDP */
+extent = RSDP_ADDRESS >> ctxt.page_shift;
+rc = populate_acpi_pages(dom, &extent, 1, &ctxt);
+if (rc) {
+LOG(ERROR, "%s: populate_acpi_pages for rsdp failed with %d",
+__FUNCTION__, rc);
+goto out;
+}
+config.rsdp = (unsigned long)xc_map_foreign_range(xch, domid,
+ ctxt.page_size,
+ PROT_READ | PROT_WRITE,
+ RSDP_ADDRESS >>
ctxt.page_shift);
I think with Anthony's on-going work you should be more flexible for all
you tables.
Not sure I understand what you mean here. You want the address
(RSDP_ADDRESS) to be a variable as opposed to a macro?
I'm still trying to wrap my head around the possible interaction between
Anthony's work and your work.
Anthony's work allows dynamically loading of firmware blobs. If you use
a fixed address, theoretically it can clash with firmware blobs among
other things libxc can load. The high address is a safe bet so that
probably won't happen in practice.
Anthony's work allows loading arbitrary blobs actually. Can you take
advantage of that mechanism as well? That is, to specify all your tables
as modules and let libxc handle actually loading them into guest
memory.
Does this make sense?
Also CC Anthony here.
My understanding of Anthony's series is that its goal was to provide an
interface to pass DSDT (and maybe some other tables) from the toolstack
to hvmloader.
Here we don't have hvmloader, we are loading the tables directly into
guest's memory.
Do you use the same hvm_start_info structure? I don't think that
structure is restricted to hvmloader.
Yes, we do. However, in PVH(v2) case it will be seen next by the guest
who will expect the tables to already be in memory. I.e. there is no
intermediate Xen component, such as hvmloader, who can load the blobs.
Having said that, I wonder whether we (both x86 and ARM) could use
Anthony's xc_dom_image.full_acpi_module instead of acpitables_blob that
Shannon's series added. (even if we can't, I think
xc_hvm_firmware_module is the right datastructure to store the blob
since it has both toolstack's virtual and guest's physical addresses).
In this case, xc_hvm_firmware_module would need to be renamed as ARM
guests are neither HVM nor PV.
FWIW, from the toolstack point of view, ARM guests is considered as PV
guest.
Regards,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v1 20/20] libxl/acpi: Build ACPI tables for HVMlite guests
On 07/11/2016 06:47 AM, Wei Liu wrote:
> On Fri, Jul 08, 2016 at 01:20:46PM -0400, Boris Ostrovsky wrote:
>> On 07/08/2016 12:07 PM, Wei Liu wrote:
>>> On Fri, Jul 08, 2016 at 10:48:52AM -0400, Boris Ostrovsky wrote:
On 07/08/2016 06:55 AM, Wei Liu wrote:
>> +
>> +/* Map page that will hold RSDP */
>> +extent = RSDP_ADDRESS >> ctxt.page_shift;
>> +rc = populate_acpi_pages(dom, &extent, 1, &ctxt);
>> +if (rc) {
>> +LOG(ERROR, "%s: populate_acpi_pages for rsdp failed with %d",
>> +__FUNCTION__, rc);
>> +goto out;
>> +}
>> +config.rsdp = (unsigned long)xc_map_foreign_range(xch, domid,
>> + ctxt.page_size,
>> + PROT_READ |
>> PROT_WRITE,
>> + RSDP_ADDRESS >>
>> ctxt.page_shift);
> I think with Anthony's on-going work you should be more flexible for all
> you tables.
Not sure I understand what you mean here. You want the address
(RSDP_ADDRESS) to be a variable as opposed to a macro?
>>> I'm still trying to wrap my head around the possible interaction between
>>> Anthony's work and your work.
>>>
>>> Anthony's work allows dynamically loading of firmware blobs. If you use
>>> a fixed address, theoretically it can clash with firmware blobs among
>>> other things libxc can load. The high address is a safe bet so that
>>> probably won't happen in practice.
>>>
>>> Anthony's work allows loading arbitrary blobs actually. Can you take
>>> advantage of that mechanism as well? That is, to specify all your tables
>>> as modules and let libxc handle actually loading them into guest
>>> memory.
>>>
>>> Does this make sense?
>>>
>>> Also CC Anthony here.
>>
>> My understanding of Anthony's series is that its goal was to provide an
>> interface to pass DSDT (and maybe some other tables) from the toolstack
>> to hvmloader.
>>
>> Here we don't have hvmloader, we are loading the tables directly into
>> guest's memory.
>>
> Do you use the same hvm_start_info structure? I don't think that
> structure is restricted to hvmloader.
Yes, we do. However, in PVH(v2) case it will be seen next by the guest
who will expect the tables to already be in memory. I.e. there is no
intermediate Xen component, such as hvmloader, who can load the blobs.
Having said that, I wonder whether we (both x86 and ARM) could use
Anthony's xc_dom_image.full_acpi_module instead of acpitables_blob that
Shannon's series added. (even if we can't, I think
xc_hvm_firmware_module is the right datastructure to store the blob
since it has both toolstack's virtual and guest's physical addresses).
-boris
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [DRAFT 1] XenSock protocol design document
> -Original Message-
[snip]
>
> # XenSocks Protocol v1
>
> ## Rationale
>
> XenSocks is a paravirtualized protocol for the POSIX socket API.
>
> The purpose of XenSocks is to allow the implementation of a specific set
> of POSIX calls to be done in a domain other than your own. It allows
> connect, accept, bind, release, listen, poll, recvmsg and sendmsg to be
> implemented in another domain.
Does the other domain have privilege over the domain issuing the POSIX calls?
[snip]
> State Machine
>
> **Front** **Back**
> XenbusStateInitialising XenbusStateInitialising
> - Query virtual device- Query backend device
> properties. identification data.
> - Setup OS device instance. |
> - Allocate and initialize the|
> request ring. V
> - Publish transport parametersXenbusStateInitWait
> that will be in effect during
> this connection.
> |
> |
> V
>XenbusStateInitialised
>
> - Query frontend transport
> parameters.
> - Connect to the request ring and
> event channel.
> |
> |
> V
> XenbusStateConnected
>
> - Query backend device properties.
> - Finalize OS virtual device
>instance.
> |
> |
> V
> XenbusStateConnected
>
> Once frontend and backend are connected, they have a shared page, which
> will is used to exchange messages over a ring, and an event channel,
> which is used to send notifications.
>
What about XenbusStateClosing and XenbusStateClosed? We're missing half the
state model here. Specifically how do individual connections get terminated if
either end moves to closing? Does either end have to wait for the other?
>
> ### Commands Ring
>
> The shared ring is used by the frontend to forward socket API calls to the
> backend. I'll refer to this ring as **commands ring** to distinguish it from
> other rings which will be created later in the lifecycle of the protocol (data
> rings). The ring format is defined using the familiar `DEFINE_RING_TYPES`
> macro
> (`xen/include/public/io/ring.h`). Frontend requests are allocated on the ring
> using the `RING_GET_REQUEST` macro.
>
> The format is defined as follows:
>
> #define XENSOCK_DATARING_ORDER 6
> #define XENSOCK_DATARING_PAGES (1 << XENSOCK_DATARING_ORDER)
> #define XENSOCK_DATARING_SIZE (XENSOCK_DATARING_PAGES <<
> PAGE_SHIFT)
>
Why a fixed size? Also, I assume DATARING should be CMDRING or somesuch here.
Plus a fixed size of *six* pages seems like a lot.
> #define XENSOCK_CONNECT0
> #define XENSOCK_RELEASE3
> #define XENSOCK_BIND 4
> #define XENSOCK_LISTEN 5
> #define XENSOCK_ACCEPT 6
> #define XENSOCK_POLL 7
>
> struct xen_xensock_request {
> uint32_t id; /* private to guest, echoed in response */
> uint32_t cmd;/* command to execute */
> uint64_t sockid; /* id of the socket */
> union {
> struct xen_xensock_connect {
> uint8_t addr[28];
> uint32_t len;
> uint32_t flags;
> grant_ref_t ref[XENSOCK_DATARING_PAGES];
> uint32_t evtchn;
> } connect;
> struct xen_xensock_bind {
> uint8_t addr[28]; /* ipv6 ready */
> uint32_t len;
> } bind;
> struct xen_xensock_accept {
> uint64_t sockid;
> grant_ref_t ref[XENSOCK_DATARING_PAGES];
> uint32_t evtchn;
> } accept;
> } u;
> };
>
Perhaps some layout diagrams for the above to avoid ABI assumptions?
> The first three fields are common for every command. Their binary layout
> is:
>
> 0 4 8 12 16
> +---+---+---+---+
> | id | cmd | sockid|
> +---+---+---+---+
>
That's a start at least :-)
> - **id** is generated by the frontend and identifies one specific request
> - **cmd** is the command requested by the frontend:
> - `XENSOCK_CONNECT`: 0
> - `XENSOCK_RELEASE`: 3
> - `XENSOCK_BIND`:4
> - `XENSOCK_LISTEN`: 5
> - `XENSOCK_ACCEPT`: 6
> - `XENSOCK_POLL`:7
> - **sockid** is generated by the frontend and identifies the socket to
> connect,
> bind, etc. A new sockid is required on `XENSOCK_CONN
[Xen-devel] [ovmf test] 96996: regressions - FAIL
flight 96996 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/96996/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-qemuu-ovmf-amd64 17 guest-start/debianhvm.repeat fail REGR. vs. 94748 test-amd64-i386-xl-qemuu-ovmf-amd64 17 guest-start/debianhvm.repeat fail REGR. vs. 94748 version targeted for testing: ovmf a00df2e5626f6aafafb9bdfd3e189402b1329530 baseline version: ovmf dc99315b8732b6e3032d01319d3f534d440b43d0 Last test of basis94748 2016-05-24 22:43:25 Z 47 days Failing since 94750 2016-05-25 03:43:08 Z 47 days 107 attempts Testing same since96996 2016-07-11 02:24:40 Z0 days1 attempts People who touched revisions under test: Anandakrishnan Loganathan Ard Biesheuvel Bi, Dandan Bret Barkelew Bruce Cran Bruce Cran Chao Zhang Cinnamon Shia Cohen, Eugene Dandan Bi Darbin Reyes david wei Eric Dong Eugene Cohen Evan Lloyd Evgeny Yakovlev Feng Tian Fu Siyuan Fu, Siyuan Gary Li Gary Lin Giri P Mudusuru Graeme Gregory Hao Wu Hegde Nagaraj P Hegde, Nagaraj P hegdenag Heyi Guo Jan D?bro? Jan Dabros Jeff Fan Jiaxin Wu Jiewen Yao Joe Zhou Jordan Justen Katie Dellaquila Laszlo Ersek Liming Gao Lu, ShifeiX A lushifex Marcin Wojtas Mark Rutland Marvin H?user Marvin Haeuser Maurice Ma Michael Zimmermann Mudusuru, Giri P Ni, Ruiyu Qiu Shumin Ruiyu Ni Ruiyu Niã Ryan Harkin Sami Mujawar Satya Yarlagadda Shannon Zhao Sriram Subramanian Star Zeng Subramanian, Sriram (EG Servers Platform SW) Sunny Wang Tapan Shah Thomas Palmer Yarlagadda, Satya P Yonghong Zhu Zhang Lubo Zhang, Chao B 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 8738 lines long.) ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [patch 52/66] arm: xen: Convert to hotplug state machine
From: Richard Cochran
Install the callbacks via the state machine and let the core invoke
the callbacks on the already online CPUs.
The get_cpu() in xen_starting_cpu() boils down to preempt_disable() since
we already know the CPU we run on. Disabling preemption shouldn't be required
here from what I see since it we don't switch CPUs while invoking the function.
Signed-off-by: Richard Cochran
Reviewed-by: Sebastian Andrzej Siewior
Cc: Stefano Stabellini
Cc: xen-de...@lists.xenproject.org
Signed-off-by: Anna-Maria Gleixner
---
arch/arm/xen/enlighten.c | 41 +++--
include/linux/cpuhotplug.h |1 +
2 files changed, 12 insertions(+), 30 deletions(-)
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -161,12 +161,11 @@ static struct notifier_block xen_pvclock
.notifier_call = xen_pvclock_gtod_notify,
};
-static void xen_percpu_init(void)
+static int xen_starting_cpu(unsigned int cpu)
{
struct vcpu_register_vcpu_info info;
struct vcpu_info *vcpup;
int err;
- int cpu = get_cpu();
/*
* VCPUOP_register_vcpu_info cannot be called twice for the same
@@ -190,7 +189,13 @@ static void xen_percpu_init(void)
after_register_vcpu_info:
enable_percpu_irq(xen_events_irq, 0);
- put_cpu();
+ return 0;
+}
+
+static int xen_dying_cpu(unsigned int cpu)
+{
+ disable_percpu_irq(xen_events_irq);
+ return 0;
}
static void xen_restart(enum reboot_mode reboot_mode, const char *cmd)
@@ -209,28 +214,6 @@ static void xen_power_off(void)
BUG_ON(rc);
}
-static int xen_cpu_notification(struct notifier_block *self,
- unsigned long action,
- void *hcpu)
-{
- switch (action) {
- case CPU_STARTING:
- xen_percpu_init();
- break;
- case CPU_DYING:
- disable_percpu_irq(xen_events_irq);
- break;
- default:
- break;
- }
-
- return NOTIFY_OK;
-}
-
-static struct notifier_block xen_cpu_notifier = {
- .notifier_call = xen_cpu_notification,
-};
-
static irqreturn_t xen_arm_callback(int irq, void *arg)
{
xen_hvm_evtchn_do_upcall();
@@ -351,16 +334,14 @@ static int __init xen_guest_init(void)
return -EINVAL;
}
- xen_percpu_init();
-
- register_cpu_notifier(&xen_cpu_notifier);
-
pv_time_ops.steal_clock = xen_stolen_accounting;
static_key_slow_inc(¶virt_steal_enabled);
if (xen_initial_domain())
pvclock_gtod_register_notifier(&xen_pvclock_gtod_notifier);
- return 0;
+ return cpuhp_setup_state(CPUHP_AP_ARM_XEN_STARTING,
+"AP_ARM_XEN_STARTING", xen_starting_cpu,
+xen_dying_cpu);
}
early_initcall(xen_guest_init);
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -52,6 +52,7 @@ enum cpuhp_state {
CPUHP_AP_KVM_STARTING,
CPUHP_AP_KVM_ARM_VGIC_STARTING,
CPUHP_AP_KVM_ARM_TIMER_STARTING,
+ CPUHP_AP_ARM_XEN_STARTING,
CPUHP_AP_LEDTRIG_STARTING,
CPUHP_AP_NOTIFY_STARTING,
CPUHP_AP_ONLINE,
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] monitor access to pages with a specific p2m_type_t
On Sun, Jul 10, 2016 at 4:50 PM, sepanta s wrote:
>
>
> On Sun, Jun 26, 2016 at 5:15 PM, sepanta s wrote:
>
>>
>>
>>
>> On Fri, Jun 24, 2016 at 8:10 PM, Tamas K Lengyel
>> wrote:
>>
>>>
>>> On Jun 24, 2016 05:19, "Razvan Cojocaru"
>>> wrote:
>>> >
>>> > On 06/24/2016 02:05 PM, George Dunlap wrote:
>>> > > On Wed, Jun 22, 2016 at 12:38 PM, sepanta s
>>> wrote:
>>> > >> Hi,
>>> > >> Is it possible to monitor the access on the pages withp2m_type_t
>>> > >> p2m_ram_shared?
>>> > >
>>> > > cc'ing Tamas and Razvan
>>> >
>>> > Thanks for the CC. Judging by the "if ( npfec.write_access && (p2mt ==
>>> > p2m_ram_shared) )" line in hvm_hap_nested_page_fault() (from
>>> > xen/arch/x86/hvm/hvm.c), I'd say it certainly looks possible. But I
>>> > don't know what the context of the question is.
>>> >
>>> >
>>> > Thanks,
>>> > Razvan
>>>
>> The question is just getting the gfn and mfn of the page which is as
>> type: p2m_ram_shared to see which pages are written and unshared.
>>
>>> Yes, p2m_ram_shared type pages can be monitored with mem_access just as
>>> normal pages. The only part that may be tricky is if you map the page into
>>> your monitoring application while the page is shared. Your handle will
>>> continue to be valid even if the page is unshared but it will continue to
>>> point to the shared page. However, even if you catch write access events to
>>> the shared page that will lead to unsharing, the mem_access notification is
>>> sent before unsharing. I just usually do unsharing myself in the mem_access
>>> callback manually for monitored pages for this reason. I might change the
>>> flow in 4.8 to send the notification after the unsharing happened to
>>> simplify this.
>>>
>>> Tamas
>>>
>> Thanks, but in mem_access , what APIs can be used to see such events ?
>>
>
> Should I mark the shared pages as rx only ?
>
>
Hi,
Is there any sample code which I can undestand how to capture the events on
the gfns which have p2m_ram_shared enabled ?
I couldn't find any ... .
I would be grateful if any help , as there is not any documents through net
to use :(
Should I just set the ring_page as the pages which are shared and mark them
read-only, then capture the write events?
BTW, I added a function called mem_sharing_notify_unshare to mem_sharing.c
and added it to __mem_sharing_unshare_page at this part:
*if ( p2m_change_type_one(d, gfn, p2m_ram_shared, p2m_ram_rw) )*
*{*
*gdprintk(XENLOG_ERR, "Could not change p2m type d %hu gfn %lx.\n", *
*d->domain_id, gfn);*
*BUG();*
*}else {*
* mem_sharing_notify_unshare(d,gfn.0);*
*}*
So by having a vm event channel listening to unsharing event, I can see the
notification in xen-access . To do so, I
have used vm_event_enable which uses HVM_PARAM_SHARING_RING_PFN .
But, when I used memshrtool to share all the pages of two vms - my vm1 and
its clone vm2 .
About 900 MB of the ram is shared but there are a lot of unshared events
happening.
When I do the sharing one more time, there is not any pages unshared as the
total number of shared pages stay the same.
Seems no unsharing is done as the number of shared pages are the same.
Does any page fault triggers when a write operation is done on a shared
page among two vms ?
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v1 02/20] acpi/hvmloader: Move acpi_info initialization out of ACPI code
CC Lars
More licence stuff. Do you have contact(s)?
Wei.
On Fri, Jul 08, 2016 at 11:57:59AM -0400, Boris Ostrovsky wrote:
> On 07/08/2016 11:50 AM, Ian Jackson wrote:
> > Konrad Rzeszutek Wilk writes ("Re: [Xen-devel] [PATCH v1 02/20]
> > acpi/hvmloader: Move acpi_info initialization out of ACPI code"):
> >> Having different licenses will invite the lawyers in the conversation
> >> which can drag things out.
> > We don't want libxl to have some confusing combination of
> > alleged-licences.
> >
> >> A quick read says one can add an exception to GPLv2 license to allow it
> >> to be linked (see
> >> https://www.gnu.org/licenses/gpl-faq.en.html#GPLIncompatibleLibs)
> >> but that would require Copyright OK from the original holders.
> >>
> >> It would be far easier to ask the copyright holders:
> > Yes. That seems to be Citrix, Intel, Sun (Oracle), IBM, and:
> >
> >> Tobias Geiger
>
>
> Do we need to get consent from companies only or (also) from individuals
> listed in the files? Which means Keir and Kamala Narasimhan (who works,
> or at least used to work) for Citrix.
>
> For IBM --- whom can we contact?
>
> >>
> >> If they would be OK making the code (this is from
> >> tools/firmware/hvmloader/acpi/acpi2_0.h) lGPL.
> > Right.
> >
> >> Or is there some other technical way around this?
> > No.
> >
> >> I can't recall whether the 'dlopen' (so runtime loading
> >> vs linking) of an GPL library is from Lesser GPL is OK.
> >> (so proprietary code linking with libxl, and libxl dlopen'ing
> >> the libacpi code').
> > This kind of attempt at licence workaround by some kind of technical
> > bodge is not legally effective.
>
> We don't build files in libacpi as a dynamic library. The object files
> are linked against whoever wants to use the functionality, just like
> what we do for libelf.
>
> -boris
>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [BUG] kernel BUG at drivers/block/xen-blkfront.c:1711
On 07/11/2016 04:50 PM, Evgenii Shatokhin wrote:
> On 06.06.2016 11:42, Dario Faggioli wrote:
>> Just Cc-ing some Linux, block, and Xen on CentOS people...
>>
>
> Ping.
>
> Any suggestions how to debug this or what might cause the problem?
>
> Obviously, we cannot control Xen on the Amazon's servers. But perhaps there
> is something we can do at the kernel's side, is it?
>
>> On Mon, 2016-06-06 at 11:24 +0300, Evgenii Shatokhin wrote:
>>> (Resending this bug report because the message I sent last week did
>>> not
>>> make it to the mailing list somehow.)
>>>
>>> Hi,
>>>
>>> One of our users gets kernel panics from time to time when he tries
>>> to
>>> use his Amazon EC2 instance with CentOS7 x64 in it [1]. Kernel panic
>>> happens within minutes from the moment the instance starts. The
>>> problem
>>> does not show up every time, however.
>>>
>>> The user first observed the problem with a custom kernel, but it was
>>> found later that the stock kernel 3.10.0-327.18.2.el7.x86_64 from
>>> CentOS7 was affected as well.
Please try this patch:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=7b0767502b5db11cb1f0daef2d01f6d71b1192dc
Regards,
Bob
>>>
>>> The part of the system log he was able to retrieve is attached. Here
>>> is
>>> the bug info, for convenience:
>>>
>>>
>>> [2.246912] kernel BUG at drivers/block/xen-blkfront.c:1711!
>>> [2.246912] invalid opcode: [#1] SMP
>>> [2.246912] Modules linked in: ata_generic pata_acpi
>>> crct10dif_pclmul
>>> crct10dif_common crc32_pclmul crc32c_intel ghash_clmulni_intel
>>> xen_netfront xen_blkfront(+) aesni_intel lrw ata_piix gf128mul
>>> glue_helper ablk_helper cryptd libata serio_raw floppy sunrpc
>>> dm_mirror
>>> dm_region_hash dm_log dm_mod scsi_transport_iscsi
>>> [2.246912] CPU: 1 PID: 50 Comm: xenwatch Not tainted
>>> 3.10.0-327.18.2.el7.x86_64 #1
>>> [2.246912] Hardware name: Xen HVM domU, BIOS 4.2.amazon
>>> 12/07/2015
>>> [2.246912] task: 8800e9fcb980 ti: 8800e98bc000 task.ti:
>>> 8800e98bc000
>>> [2.246912] RIP: 0010:[] []
>>> blkfront_setup_indirect+0x41f/0x430 [xen_blkfront]
>>> [2.246912] RSP: 0018:8800e98bfcd0 EFLAGS: 00010283
>>> [2.246912] RAX: 8800353e15c0 RBX: 8800e98c52c8 RCX:
>>> 0020
>>> [2.246912] RDX: 8800353e15b0 RSI: 8800e98c52b8 RDI:
>>> 8800353e15d0
>>> [2.246912] RBP: 8800e98bfd20 R08: 8800353e15b0 R09:
>>> 8800eb403c00
>>> [2.246912] R10: a0155532 R11: ffe8 R12:
>>> 8800e98c4000
>>> [2.246912] R13: 8800e98c52b8 R14: 0020 R15:
>>> 8800353e15c0
>>> [2.246912] FS: () GS:8800efc2()
>>> knlGS:
>>> [2.246912] CS: 0010 DS: ES: CR0: 80050033
>>> [2.246912] CR2: 7f1b615ef000 CR3: e2b44000 CR4:
>>> 001406e0
>>> [2.246912] DR0: DR1: DR2:
>>>
>>> [2.246912] DR3: DR6: 0ff0 DR7:
>>> 0400
>>> [2.246912] Stack:
>>> [2.246912] 0020 0001 0020a0157217
>>> 0100e98bfdbc
>>> [2.246912] 27efa3ef 8800e98bfdbc 8800e98ce000
>>> 8800e98c4000
>>> [2.246912] 8800e98ce040 0001 8800e98bfe08
>>> a0155d4c
>>> [2.246912] Call Trace:
>>> [2.246912] [] blkback_changed+0x4ec/0xfc8
>>> [xen_blkfront]
>>> [2.246912] [] ? xenbus_gather+0x170/0x190
>>> [2.246912] [] ? __slab_free+0x10e/0x277
>>> [2.246912] []
>>> xenbus_otherend_changed+0xad/0x110
>>> [2.246912] [] ? xenwatch_thread+0x77/0x180
>>> [2.246912] [] backend_changed+0x13/0x20
>>> [2.246912] [] xenwatch_thread+0x66/0x180
>>> [2.246912] [] ? wake_up_atomic_t+0x30/0x30
>>> [2.246912] [] ?
>>> unregister_xenbus_watch+0x1f0/0x1f0
>>> [2.246912] [] kthread+0xcf/0xe0
>>> [2.246912] [] ?
>>> kthread_create_on_node+0x140/0x140
>>> [2.246912] [] ret_from_fork+0x58/0x90
>>> [2.246912] [] ?
>>> kthread_create_on_node+0x140/0x140
>>> [2.246912] Code: e1 48 85 c0 75 ce 49 8d 84 24 40 01 00 00 48 89
>>> 45
>>> b8 e9 91 fd ff ff 4c 89 ff e8 8d ae 06 e1 e9 f2 fc ff ff 31 c0 e9 2e
>>> fe
>>> ff ff <0f> 0b e8 9a 57 f2 e0 0f 0b 0f 1f 84 00 00 00 00 00 0f 1f 44
>>> 00
>>> [2.246912] RIP []
>>> blkfront_setup_indirect+0x41f/0x430 [xen_blkfront]
>>> [2.246912] RSP
>>> [2.491574] ---[ end trace 8a9b992812627c71 ]---
>>> [2.495618] Kernel panic - not syncing: Fatal exception
>>>
>>>
>>> Xen version 4.2.
>>>
>>> EC2 instance type: c3.large with EBS magnetic storage, if that
>>> matters.
>>>
>>> Here is the code where the BUG_ON triggers (drivers/block/xen-
>>> blkfront.c):
>>>
>>> if (!info->feature_persistent && info->max_indirect_segments) {
>>> /
Re: [Xen-devel] [PATCH 3/8] x86/time: introduce and use rdtsc_ordered()
On Mon, 2016-06-20 at 13:59 +0100, Andrew Cooper wrote:
> On 15/06/16 11:27, Jan Beulich wrote:
> > Matching Linux commit 03b9730b76 ("x86/asm/tsc: Add rdtsc_ordered()
> > and
> > use it in trivial call sites") and earlier ones it builds upon,
> > let's
> > make sure timing loops don't have their rdtsc()-s re-ordered, as
> > that
> > would harm precision of the result (values were observed to be
> > several
> > hundred clocks off without this adjustment).
> >
> > Signed-off-by: Jan Beulich
>
> Reviewed-by: Andrew Cooper
>
FWIW:
Reviewed-by: Dario Faggioli
Tested-by: Dario Faggioli
(or Reviewed-and-Tested-by: as you wish :-)).
FTR, during my own investigation, before raising the issue on the
mailing list, I also came to the conclusion that we'd need something
like this. I even try doing something like this (in a much more hacky
way), and had the feeling that it was making a difference but, of
course, alone, without all the other issues that Jan found and fixed in
this series, it wasn't enough.
Thanks and regards,
Dario
--
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 5/5] libxl: only issue cpu-add call to QEMU for not present CPU
On Fri, Jul 08, 2016 at 06:48:27PM +0100, Ian Jackson wrote:
> Wei Liu writes ("[PATCH v3 5/5] libxl: only issue cpu-add call to QEMU for
> not present CPU"):
> > Calculate the final bitmap for CPUs to add to avoid having annoying
> > error messages complaining those CPUs are already present.
>
> Can this be expanded a bit ? I think perhaps it would benefit from
> "When X and Y and Z, qemu prints an annoying message about Q".
>
> I think I have understood it by reverse engineering the code but I
> would still like a better commit message.
Sure. I can paste in the error message into commit log.
Wei.
>
> Thanks,
> Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 4/5] libxl: update vcpus bitmap in retrieved guest config
On Fri, Jul 08, 2016 at 06:44:28PM +0100, Ian Jackson wrote:
> Wei Liu writes ("[PATCH v3 4/5] libxl: update vcpus bitmap in retrieved guest
> config"):
> > ... because the available vcpu bitmap can change during domain life time
> > due to cpu hotplug and unplug.
> >
> > For QEMU upstream, we interrogate QEMU for the number of vcpus. For
> > others, we look directly into xenstore for information.
> ...
> > +static int libxl__update_avail_vcpus_qmp(libxl__gc *gc, uint32_t domid,
> > + unsigned int max_vcpus,
> > + libxl_bitmap *map)
> > +{
> > +int rc;
> > +
> > +/* For QEMU upstream we always need to return the number
> > + * of cpus present to QEMU whether they are online or not;
> > + * otherwise QEMU won't accept the saved state.
>
> This comment is confusing to me. Perhaps `return' should read
> `provide' ? But the connection between the body of this function and
> the comment is still obscure.
>
"provide" is ok.
To avoid confusion the comment can be moved a few line above to describe
the function, not the code snippet. Does that work better?
> > +static int libxl__update_avail_vcpus_xenstore(libxl__gc *gc, uint32_t
> > domid,
> > + unsigned int max_vcpus,
> > + libxl_bitmap *map)
> > +{
> > +int rc;
> > +unsigned int i;
> > +const char *dompath;
> ...
> > +for (i = 0; i < max_vcpus; i++) {
> > +const char *path = GCSPRINTF("%s/cpu/%u/availability", dompath, i);
> > +const char *content = libxl__xs_read(gc, XBT_NULL, path);
> > +if (!strcmp(content, "online"))
> > +libxl_bitmap_set(map, i);
>
> Firstly, this should be libxl__xs_read_checked. Secondly if "content"
> is NULL, this will crash.
>
Will fix.
> > @@ -7294,6 +7341,55 @@ int libxl_retrieve_domain_configuration(libxl_ctx
> > *ctx, uint32_t domid,
> > libxl_dominfo_dispose(&info);
> > }
> >
> > +/* VCPUs */
> > +{
> > +libxl_bitmap *map = &d_config->b_info.avail_vcpus;
> > +unsigned int max_vcpus = d_config->b_info.max_vcpus;
> > +libxl_device_model_version version;
> > +
> > +libxl_bitmap_dispose(map);
> > +libxl_bitmap_init(map);
> > +libxl_bitmap_alloc(CTX, map, max_vcpus);
> > +libxl_bitmap_set_none(map);
>
> I see that this function already trashes the domain config when it
> fails (leaving it up to the caller to free the partial config) but
> that this is not documented :-/.
d_config is an out parameter. User can't expect the returned d_config to
be valid if this API returns error state. And user is still responsible
for calling _init before calling this API and _dispose afterwards. So
this still fits into how we expect libxl types to be used. Nothing new
needs to be documented.
All other fields are already handled in this way.
>
> > +/* If the user did not explicitly specify a device model
> > + * version, we need to use the same logic used during domain
> > + * creation to derive the device model version.
> > + */
> > +version = d_config->b_info.device_model_version;
> > +if (version == LIBXL_DEVICE_MODEL_VERSION_UNKNOWN)
> > +version = libxl__get_device_model_version(gc,
> > &d_config->b_info);
>
> I think this is rather unfortunate. Won't changing the default device
> model while the domain is running will cause this function to give
> wrong answers ?
I think saving the device model into the template should work. No need
to derive it during runtime.
Wei.
>
> Thanks,
> Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [xen-unstable test] 96993: tolerable FAIL
flight 96993 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/96993/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-armhf-armhf-libvirt-xsm 7 host-ping-check-xen fail in 96893 pass in 96993 test-amd64-amd64-i386-pvgrub 18 guest-start/debian.repeat fail in 96893 pass in 96993 test-armhf-armhf-xl-credit2 4 host-ping-check-native fail pass in 96893 test-armhf-armhf-xl-arndale 6 xen-bootfail pass in 96893 Regressions which are regarded as allowable (not blocking): build-i386-rumpuserxen6 xen-buildfail like 96893 build-amd64-rumpuserxen 6 xen-buildfail like 96893 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 96893 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail like 96893 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 96893 test-amd64-amd64-xl-rtds 9 debian-install fail like 96893 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 96893 Tests which did not succeed, but are not blocking: test-amd64-amd64-rumpuserxen-amd64 1 build-check(1) blocked n/a test-amd64-i386-rumpuserxen-i386 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit2 13 saverestore-support-check fail in 96893 never pass test-armhf-armhf-xl-credit2 12 migrate-support-check fail in 96893 never pass test-armhf-armhf-xl-arndale 12 migrate-support-check fail in 96893 never pass test-armhf-armhf-xl-arndale 13 saverestore-support-check fail in 96893 never pass 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-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-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-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-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-libvirt 14 guest-saverestorefail never pass test-armhf-armhf-libvirt 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-libvirt-qcow2 11 migrate-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail 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-rtds 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 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 version targeted for testing: xen 7da483b0236d8974cc97f81780dcf8e559a63175 baseline version: xen 7da483b0236d8974cc97f81780dcf8e559a63175 Last test of basis96993 2016-07-11 01:58:23 Z0 days Testing same since0 1970-01-01 00:00:00 Z 16993 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-
Re: [Xen-devel] [PATCH v3 1/5] libxl: libxl_domain_need_memory shouldn't modify b_info
On Fri, Jul 08, 2016 at 06:35:59PM +0100, Ian Jackson wrote:
> Wei Liu writes ("[PATCH v3 1/5] libxl: libxl_domain_need_memory shouldn't
> modify b_info"):
> > This function is used to return the memory needed for a guest. It's not
> > in a position to modify the b_info passed in (note the _setdefault
> > function).
> >
> > Use a copy of b_info to do the calculation. Define a macro to mark the
> > change in API.
>
> Urgh, how unpleasant.
>
> > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> > index 5ec4c80..65af9ee 100644
> > --- a/tools/libxl/libxl.c
> > +++ b/tools/libxl/libxl.c
> > @@ -5123,20 +5123,24 @@ int libxl_domain_need_memory(libxl_ctx *ctx,
> > libxl_domain_build_info *b_info,
> > uint32_t *need_memkb)
> > {
> > GC_INIT(ctx);
> > +libxl_domain_build_info tmp;
>
> I would suggest, instead:
>
> int libxl_domain_need_memory(libxl_ctx *ctx,
> - libxl_domain_build_info *b_info,
> + const libxl_domain_build_info *b_info_in,
> ...
> + libxl_domain_build_info b_info[1];
>
> If you do this then you do not need to change the bulk of the body of
> the function at all.
>
This is a good idea.
Just that I discover there is another unpleasant fact: all the generated
copy function doesn't constify their source parameter. I will need to
write a patch to fix that first.
Wei.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [DRAFT 1] XenSock protocol design document
On Fri, 8 Jul 2016, David Vrabel wrote: > On 08/07/16 12:23, Stefano Stabellini wrote: > > > > XenSocks provides the following benefits: > > * guest networking works out of the box with VPNs, wireless networks and > > any other complex configurations on the host > > Only in the trivial case where the host only has one external network. Which is the most common case and the one we care about the most. > Otherwise, you are going to have to have some sort of configuration to > keep guest traffic isolated from the management or storage network (for > example). I admit I don't think I understand your example, please add more details. In any case how would you achieve this benefit with netfront/netback? > > * guest services listen on ports bound directly to the backend domain IP > > addresses > > I think this could be done with SDN but I'm no expert on this area. Maybe. But a simple Google search didn't turn up anything useful on this. The solution used by Docker to achieve this is very expensive in terms of resources. In fact even if you are right, these are complex and expensive solutions you are talking about. It would likely require some sort of address and port translation. XenSock is a simple solution and the best way to solve this problem. I don't want to configure an SDN, iptables and whatnot just to have guest ports bound on the host. More complexity one introduces, the more difficult becomes handling security and maintenance. Performance suffers too. > > * localhost becomes a secure namespace for intra-VMs communications > > I presume you mean "inter-VM" communication here? Yes, I meant inter-VM, sorry for the confusion. > This is already achievable with a private bridged network for VMs on a > host. Wouldn't that require one more virtual interface per VM? > > * full visibility of the guest behavior on the backend domain, allowing > > for inexpensive filtering and manipulation of any guest calls > > There's many existing solutions in this space for networking. One the most important points of this work is that users don't need to use those "existing solutions in this space for networking". They are expensive (both in terms of money and performance) and suboptimal. They are never going to have the level of visibility and control that we could have with XenSock. > > * excellent performance > > netback/netfront is pretty good now and further improvements to them > would have wider benefits. You are saying that one could achieve the same benefits of XenSock with: netfront/netback + some zero configuration tool for netfront/netback + SDN + a network based application firewall + one more virtual interface per VM I feel confortable in stating that XenSock is far better than a combination of 4 or 5 complex moving pieces. I admit that Citrix XenServer won't directly benefit from XenSock, at least not in the short term. But the Xen Community is much wider than XenServer. People have already pointed out to me why this would be useful for them. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] libxl: trigger attach events for devices attached before xl devd startup
On Mon, Jul 11, 2016 at 12:44:42PM +0200, Marek Marczykowski-Górecki wrote: > When this daemon is started after creating backend device, that device > will not be configured. > > Racy situation: > 1. driver domain is started > 2. frontend domain is started (just after kicking driver domain off) > 3. device in frontend domain is connected to the backend (as specified >in frontend domain configuration) > 4. xl devd is started in driver domain > > End result is that backend device in driver domain is not configured > (like network interface is not enabled), so the device doesn't work. > > Fix this by artifically triggering events for devices already present in > xenstore before xl devd is started. Do this only after xenstore watch is > already registered, and only for devices not already initialized (in > XenbusStateInitWait state). > > Cc: Ian Jackson > Cc: Wei Liu > Signed-off-by: Marek Marczykowski-Górecki Thanks, this looks fine to me: Acked-by: Roger Pau Monné Roger. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 0/4] libxl: add framework for device types
On Fri, Jul 08, 2016 at 06:54:54PM +0100, Ian Jackson wrote:
> Juergen Gross writes ("[PATCH v2 0/4] libxl: add framework for device types"):
> > Instead of duplicate coding for each device type (vtpms, usbctrls, ...)
> > especially on domain creation introduce a framework for that purpose.
> >
> > I especially found it annoying that e.g. the vtpm callback issued the
> > error message for a failed attach of nic devices.
>
> I was tempted to just commit this, having acked all four, but given
> the nature of this series I'm going to wait a bit for further comments
> :-).
>
This series looks good to me.
Wei.
> Regards,
> Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v1 20/20] libxl/acpi: Build ACPI tables for HVMlite guests
On Fri, Jul 08, 2016 at 01:20:46PM -0400, Boris Ostrovsky wrote:
> On 07/08/2016 12:07 PM, Wei Liu wrote:
> > On Fri, Jul 08, 2016 at 10:48:52AM -0400, Boris Ostrovsky wrote:
> >> On 07/08/2016 06:55 AM, Wei Liu wrote:
> +
> +/* Map page that will hold RSDP */
> +extent = RSDP_ADDRESS >> ctxt.page_shift;
> +rc = populate_acpi_pages(dom, &extent, 1, &ctxt);
> +if (rc) {
> +LOG(ERROR, "%s: populate_acpi_pages for rsdp failed with %d",
> +__FUNCTION__, rc);
> +goto out;
> +}
> +config.rsdp = (unsigned long)xc_map_foreign_range(xch, domid,
> + ctxt.page_size,
> + PROT_READ |
> PROT_WRITE,
> + RSDP_ADDRESS >>
> ctxt.page_shift);
> >>> I think with Anthony's on-going work you should be more flexible for all
> >>> you tables.
> >> Not sure I understand what you mean here. You want the address
> >> (RSDP_ADDRESS) to be a variable as opposed to a macro?
> >>
> > I'm still trying to wrap my head around the possible interaction between
> > Anthony's work and your work.
> >
> > Anthony's work allows dynamically loading of firmware blobs. If you use
> > a fixed address, theoretically it can clash with firmware blobs among
> > other things libxc can load. The high address is a safe bet so that
> > probably won't happen in practice.
> >
> > Anthony's work allows loading arbitrary blobs actually. Can you take
> > advantage of that mechanism as well? That is, to specify all your tables
> > as modules and let libxc handle actually loading them into guest
> > memory.
> >
> > Does this make sense?
> >
> > Also CC Anthony here.
>
>
> My understanding of Anthony's series is that its goal was to provide an
> interface to pass DSDT (and maybe some other tables) from the toolstack
> to hvmloader.
>
> Here we don't have hvmloader, we are loading the tables directly into
> guest's memory.
>
Do you use the same hvm_start_info structure? I don't think that
structure is restricted to hvmloader.
Wei.
> -boris
>
>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2] libxl: trigger attach events for devices attached before xl devd startup
When this daemon is started after creating backend device, that device
will not be configured.
Racy situation:
1. driver domain is started
2. frontend domain is started (just after kicking driver domain off)
3. device in frontend domain is connected to the backend (as specified
in frontend domain configuration)
4. xl devd is started in driver domain
End result is that backend device in driver domain is not configured
(like network interface is not enabled), so the device doesn't work.
Fix this by artifically triggering events for devices already present in
xenstore before xl devd is started. Do this only after xenstore watch is
already registered, and only for devices not already initialized (in
XenbusStateInitWait state).
Cc: Ian Jackson
Cc: Wei Liu
Signed-off-by: Marek Marczykowski-Górecki
---
tools/libxl/libxl.c | 33 +
1 file changed, 33 insertions(+)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 1c81239..dd20e29 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -4743,6 +4743,12 @@ int libxl_device_events_handler(libxl_ctx *ctx,
uint32_t domid;
libxl__ddomain ddomain;
char *be_path;
+char **kinds = NULL, **domains = NULL, **devs = NULL;
+const char *sstate;
+char *state_path;
+int state;
+unsigned int nkinds, ndomains, ndevs;
+int i, j, k;
ddomain.ao = ao;
LIBXL_SLIST_INIT(&ddomain.guests);
@@ -4762,6 +4768,33 @@ int libxl_device_events_handler(libxl_ctx *ctx,
be_path);
if (rc) goto out;
+kinds = libxl__xs_directory(gc, XBT_NULL, be_path, &nkinds);
+if (kinds) {
+for (i = 0; i < nkinds; i++) {
+domains = libxl__xs_directory(gc, XBT_NULL,
+GCSPRINTF("%s/%s", be_path, kinds[i]), &ndomains);
+if (!domains)
+continue;
+for (j = 0; j < ndomains; j++) {
+devs = libxl__xs_directory(gc, XBT_NULL,
+GCSPRINTF("%s/%s/%s", be_path, kinds[i], domains[j]),
&ndevs);
+if (!devs)
+continue;
+for (k = 0; k < ndevs; k++) {
+state_path = GCSPRINTF("%s/%s/%s/%s/state",
+be_path, kinds[i], domains[j], devs[k]);
+rc = libxl__xs_read_checked(gc, XBT_NULL, state_path,
&sstate);
+if (rc)
+continue;
+state = atoi(sstate);
+if (state == XenbusStateInitWait)
+backend_watch_callback(egc, &ddomain.watch,
+be_path, state_path);
+}
+}
+}
+}
+
return AO_INPROGRESS;
out:
--
2.5.5
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [BUG] kernel BUG at drivers/block/xen-blkfront.c:1711
On Mon, Jul 11, 2016 at 9:50 AM, Evgenii Shatokhin wrote: > On 06.06.2016 11:42, Dario Faggioli wrote: >> >> Just Cc-ing some Linux, block, and Xen on CentOS people... >> > > Ping. > > Any suggestions how to debug this or what might cause the problem? > > Obviously, we cannot control Xen on the Amazon's servers. But perhaps there > is something we can do at the kernel's side, is it? I think part of the problem is that your report has confused people so that everyone cc'd thinks it's someone else's problem. :-) To wit.. >>> One of our users gets kernel panics from time to time when he tries >>> to >>> use his Amazon EC2 instance with CentOS7 x64 in it [1]. Kernel panic >>> happens within minutes from the moment the instance starts. The >>> problem >>> does not show up every time, however. >>> >>> The user first observed the problem with a custom kernel, but it was >>> found later that the stock kernel 3.10.0-327.18.2.el7.x86_64 from >>> CentOS7 was affected as well. ...by mentioning the exact CentOS kernel version, but not the version of the "custom kernel" you used, I suspect the people familiar with netfront filtered this out as something to be taken care of by the CentOS / RHEL system. If you can reproduce this with a relatively recent stock kernel, please post the kernel version and the debug information. If you can't, then it's likely to be an issue that RH needs to take care of by backporting whatever change fixed the issue. Thanks, -George ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 6/7] libxenstat: honour XEN_RUN_DIR
On Fri, Jul 08, 2016 at 06:27:28PM +0100, Ian Jackson wrote:
> Wei Liu writes ("[PATCH 6/7] libxenstat: honour XEN_RUN_DIR"):
> > This is because libxl uses XEN_RUN_DIR to generate the socket path for
> > libxenstat while libxenstat itself uses hard-coded path, which is not
> > necessary in sync with libxl.
>
which is not necessarily the same path as XEN_RUN_DIR.
> This commit message is confusing (and perhaps ungrammatical). Is
> there currently a bug here ?
>
> Maybe this patch or something like it is a backport candidate ?
>
There is no bug in the default configuration because XEN_RUN_DIR is
/var/run/xen by default. But if users chooses a different path the path
used in libxl and the path used in libxenstat will go out of sync hence
rendering libxenstat useless.
I think this is a backport candidate.
Wei.
> Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 7/7] oxenstored: honour XEN_RUN_DIR
On Fri, Jul 08, 2016 at 06:28:40PM +0100, Ian Jackson wrote:
> Ian Jackson writes ("Re: [PATCH 7/7] oxenstored: honour XEN_RUN_DIR"):
> > Wei Liu writes ("[PATCH 7/7] oxenstored: honour XEN_RUN_DIR"):
> > > Move default the pid file under XEN_RUN_DIR. Note that it changes the
> > > location from /var/run to /var/run/xen.
> >
> > Acked-by: Ian Jackson
>
> Actually, shouldn't this be combined with some init script patch, as
> otherwise the tree is not bisectable ?
>
There is no init script to modify because oxenstored uses these paths in
code by default.
Wei.
> Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 2/7] tools/helper: honour XEN_RUN_DIR in init-xenstore-domain.c
On Fri, Jul 08, 2016 at 06:25:13PM +0100, Ian Jackson wrote:
> Wei Liu writes ("[PATCH 2/7] tools/helper: honour XEN_RUN_DIR in
> init-xenstore-domain.c"):
> > Place the PID file under XEN_RUN_DIR. Note that this change the default
> > location from /var/run to /var/run/xen.
> >
> > Generate a _paths.h as that is required to make this change work.
> ...
> > -init-xenstore-domain: $(INIT_XENSTORE_DOMAIN_OBJS)
> > +init-xenstore-domain: $(INIT_XENSTORE_DOMAIN_OBJS) _paths.h
>
> This dependency is surely wrong. It is the object(s) (ie, the
> compile) that depend on _paths.h, not the executable (ie the link).
>
Good catch. I will fix it in next version.
Wei.
> But otherwise this is fine.
>
> Thanks,
> Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] xen: arm64: Add support for Renesas RCar Gen3 H3 Salvator-X platform
Hi, On 11.07.2016 11:25, Andre Przywara wrote: Hi Dirk, On 08/07/16 12:38, Dirk Behme wrote: Hi Andre, On 05.07.2016 16:29, Andre Przywara wrote: Hi, On 05/07/16 15:22, Dirk Behme wrote: On 05.07.2016 15:45, Andre Przywara wrote: Hi, On 05/07/16 14:34, Julien Grall wrote: (CC Andre) On 05/07/16 14:04, Dirk Behme wrote: On 05.07.2016 14:45, Julien Grall wrote: On 05/07/16 13:09, Dirk Behme wrote: Hi Julien, On 05.07.2016 13:39, Julien Grall wrote: Hi Dirk, On 05/07/16 07:37, Dirk Behme wrote: Signed-off-by: Dirk Behme This patch looks good to me, however I would like to see the documentation on the wiki page (see [1]) before giving any formal ack. Ok, many thanks for your review! Yes, I understood that something more is needed. I just wonder if we keep the patch on the mailing list for a moment. With this it's publically available and we can see how's the public interest for this board. Additionally, getting Xen running on this board and describe all this in the wiki isn't that easy. You either need to modify the flashed firmware. Or, like me, load everything via a JTAG debugger ... Can you details why you think it is not easy? Why do you have to modify the firmware? Is it because it does not boot the hypervisor in EL2? The board boots via ATF, which starts U-Boot in EL1, then. You have to find a way to load Xen into memory (e.g. U-Boot TFTP) and then switch to EL2 to start Xen. But ATF is running in EL3, right? If so, can ATF just starts U-boot in EL2? I have a board at home (pine64) which also uses ATF and U-Boot and is able to boot Xen without any SMC call because the U-Boot is entered in EL2. From having a very quick look into the rcar ATF port on github[1] I see: +#elif (RCAR_BL33_EXECUTION_EL == BL33_EL2) +return (uint32_t)SPSR_64(MODE_EL2, MODE_SP_ELX, DISABLE_ALL_EXCEPTIONS); So if you rebuild ATF with RCAR_BL33_EXECUTION_EL set to BL33_EL2 that should enter U-Boot in EL2. The fix for the Pine64 was equally simple and works fine since then. This is an other solution most probably I meant when talking about "modifying the firmware" ;) I'm somehow unsure about the pros and cons running U-Boot at EL1 versus EL2, though. It shouldn't matter, really. U-Boot on AArch64 (called armv8 here) is able to run in any exception level (in contrast to ARMv7). Actually running in EL2 is the architecturally recommended way, even with ATF (by default it drops into EL2 when returning to non-secure world). On Juno (and the Pine64) is works fine this way. As U-Boot only uses an identity mapping, many differences between EL2 and EL1 don't matter. An understanding question regarding this: We found that running U-Boot at EL2 works fine and starting Xen works fine, then, too. So many thanks for that hint! But we found that the Linux native boot case (without Xen) does fail, then. I.e. running U-Boot in EL1 starts the native Linux kernel successfully. While running U-Boot in EL2 starting the native Linux kernel fails, then. That's rather odd, can you provide some debug information? Any idea regarding this? Who is responsible to switch to EL1 for the native Linux boot case if U-Boot runs at EL2? That's well supported kernel code in arch/arm64/kernel/head.S. If it sees the kernel to be entered in EL2 (which is highly recommended, btw), it installs the KVM HYP stub and drops into EL1 (after some register setup). You might want to check the code in there to see if something springs to mind or use some debug UART output to see where it gets stuck (if it gets stuck). I usually do something like "mov x1, #UART_TR; mov w0, #'1'; str w0, [x1]" to get an idea where low level code hangs. Just for the logs, it seems that the kernel I was using missed https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/arch/arm64/boot/dts/renesas?id=4c811edf65e809e6ac6ec35f4818efba2b1c6163 resulting in EL2 interrupt misbehavior. Thanks! Best regards Dirk ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl: trigger attach events for devices attached before xl devd startup
On Mon, Jul 11, 2016 at 11:49:19AM +0200, Marek Marczykowski-Górecki wrote:
> On Mon, Jul 11, 2016 at 11:43:18AM +0200, Roger Pau Monné wrote:
> > On Mon, Jul 11, 2016 at 10:56:04AM +0200, Marek Marczykowski-Górecki wrote:
> > > On Mon, Jul 11, 2016 at 10:31:17AM +0200, Roger Pau Monné wrote:
> > > > On Sun, Jul 10, 2016 at 07:35:47PM +0200, Marek Marczykowski-Górecki
> > > > wrote:
> > > > > When this daemon is started after creating backend device, that device
> > > > > will not be configured.
> > > > >
> > > > > Racy situation:
> > > > > 1. driver domain is started
> > > > > 2. frontend domain is started (just after kicking driver domain off)
> > > > > 3. device in frontend domain is connected to the backend (as specified
> > > > >in frontend domain configuration)
> > > > > 4. xl devd is started in driver domain
> > > > >
> > > > > End result is that backend device in driver domain is not configured
> > > > > (like network interface is not enabled), so the device doesn't work.
> > > > >
> > > > > Fix this by artifically triggering events for devices already present
> > > > > in
> > > > > xenstore before xl devd is started. Do this only after xenstore watch
> > > > > is
> > > > > already registered, and only for devices not already initialized (in
> > > > > XenbusStateInitWait state).
> > > >
> > > > Thanks!
> > > >
> > > > > Cc: Ian Jackson
> > > > > Cc: Wei Liu
> > > > > Signed-off-by: Marek Marczykowski-Górecki
> > > > >
> > > > > ---
> > > > > tools/libxl/libxl.c | 40
> > > > > 1 file changed, 40 insertions(+)
> > > > >
> > > > > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> > > > > index 1c81239..99815a7 100644
> > > > > --- a/tools/libxl/libxl.c
> > > > > +++ b/tools/libxl/libxl.c
> > > > > @@ -4743,8 +4743,16 @@ int libxl_device_events_handler(libxl_ctx *ctx,
> > > > > uint32_t domid;
> > > > > libxl__ddomain ddomain;
> > > > > char *be_path;
> > > > > +char **kinds = NULL, **domains = NULL, **devs = NULL;
> > > > > +const char *sstate;
> > > > > +char *state_path;
> > > > > +int state;
> > > > > +unsigned int nkinds, ndomains, ndevs;
> > > > > +int i, j, k;
> > > > > +xs_transaction_t t;
> > > > >
> > > > > ddomain.ao = ao;
> > > > > +FILLZERO(ddomain.watch);
> > > >
> > > > Is this a different bugfix or stray change?
> > >
> > > To cleanly unregister watch and not do nothing if wasn't registered at
> > > all. If it isn't initialized, libxl__ev_xswatch_deregister call on
> > > not registered watch isn't harmless.
> >
> > Right, I've realized that before your changes the function only registered
> > the watch and exited, this is needed now.
> >
> > > > > LIBXL_SLIST_INIT(&ddomain.guests);
> > > > >
> > > > > rc = libxl__get_domid(gc, &domid);
> > > > > @@ -4762,9 +4770,41 @@ int libxl_device_events_handler(libxl_ctx *ctx,
> > > > > be_path);
> > > > > if (rc) goto out;
> > > > >
> > > > > +rc = libxl__xs_transaction_start(gc, &t);
> > > > > +if (rc) goto out;
> > > >
> > > > Why do you need to start a transaction here if you end up aborting it
> > > > when
> > > > finished?
> > >
> > > Mostly to ease error checking. Because below code does three level
> > > listing, I don't want to deal with races where some entry was removed
> > > between those calls, at least not here. Like this:
> > >
> > > xs_directory('backend/vif') -> 3, 4, 5
> > > xs_directory('backend/vif/3') -> 0, 1
> > > xs_read('backend/vif/3/0/state') -> ...
> > > xs_read('backend/vif/3/1/state') -> ...
> > > toolstack removes backend/vif/4 here
> > > xs_directory('backend/vif/4') -> fail
> > >
> > > Of course backend_watch_callback would fail anyway in such a case, which
> > > is ok. But having snapshot of xenstore during this multi-level listing
> > > looks like avoiding some corner cases during listing itself.
> >
> > AFAICT your code seems to be prepared to deal with entries disappearing in
> > the middle of the tree walk, so I would just remove the transaction.
>
> Actually I'm considering changing error handling below to "goto out"
> instead of "continue", as race condition should be eliminated by the
> transaction, other errors (permission denied for example) maybe should
> be considered fatal. So, IMO there two options:
> - ignore failed reads and remove transaction
> - error on failed reads and keep transaction
>
> Which one would be better?
IMHO, I think the first option is better, and you already have it coded.
Roger.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen_pvscsi: reclaim the ring request when mapping data failed
On 11/07/16 11:50, David Vrabel wrote:
> On 11/07/16 10:33, Juergen Gross wrote:
>> On 11/07/16 04:51, Bin Wu wrote:
>>> During scsi command queueing, if mapping data fails, we need to
>>> reclaim the failed request. Otherwise, the garbage request will
>>> be pushed into the ring for the backend to work.
>>
>> Well spotted. There is another instance of this problem in
>> scsifront_action_handler(). Would you mind correcting this one, too?
>
> Would it make more sense to advance req_prod_pvt only if the request has
> been successfully created?
Yeah, probably as the first action in scsifront_do_request().
Juergen
>
> David
>
>>> Signed-off-by: Bin Wu
>>> ---
>>> drivers/scsi/xen-scsifront.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/scsi/xen-scsifront.c b/drivers/scsi/xen-scsifront.c
>>> index 9dc8687..655163d 100644
>>> --- a/drivers/scsi/xen-scsifront.c
>>> +++ b/drivers/scsi/xen-scsifront.c
>>> @@ -565,6 +565,7 @@ static int scsifront_queuecommand(struct Scsi_Host
>>> *shost,
>>> err = map_data_for_request(info, sc, ring_req, shadow);
>>> if (err < 0) {
>>> pr_debug("%s: err %d\n", __func__, err);
>>> +info->ring.req_prod_pvt--;
>>> scsifront_put_rqid(info, rqid);
>>> scsifront_return(info);
>>> spin_unlock_irqrestore(shost->host_lock, flags);
>>
>>
>> ___
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> https://lists.xen.org/xen-devel
>>
>
>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen_pvscsi: reclaim the ring request when mapping data failed
On 11/07/16 10:33, Juergen Gross wrote:
> On 11/07/16 04:51, Bin Wu wrote:
>> During scsi command queueing, if mapping data fails, we need to
>> reclaim the failed request. Otherwise, the garbage request will
>> be pushed into the ring for the backend to work.
>
> Well spotted. There is another instance of this problem in
> scsifront_action_handler(). Would you mind correcting this one, too?
Would it make more sense to advance req_prod_pvt only if the request has
been successfully created?
David
>> Signed-off-by: Bin Wu
>> ---
>> drivers/scsi/xen-scsifront.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/scsi/xen-scsifront.c b/drivers/scsi/xen-scsifront.c
>> index 9dc8687..655163d 100644
>> --- a/drivers/scsi/xen-scsifront.c
>> +++ b/drivers/scsi/xen-scsifront.c
>> @@ -565,6 +565,7 @@ static int scsifront_queuecommand(struct Scsi_Host
>> *shost,
>> err = map_data_for_request(info, sc, ring_req, shadow);
>> if (err < 0) {
>> pr_debug("%s: err %d\n", __func__, err);
>> +info->ring.req_prod_pvt--;
>> scsifront_put_rqid(info, rqid);
>> scsifront_return(info);
>> spin_unlock_irqrestore(shost->host_lock, flags);
>
>
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl: trigger attach events for devices attached before xl devd startup
On Mon, Jul 11, 2016 at 11:43:18AM +0200, Roger Pau Monné wrote:
> On Mon, Jul 11, 2016 at 10:56:04AM +0200, Marek Marczykowski-Górecki wrote:
> > On Mon, Jul 11, 2016 at 10:31:17AM +0200, Roger Pau Monné wrote:
> > > On Sun, Jul 10, 2016 at 07:35:47PM +0200, Marek Marczykowski-Górecki
> > > wrote:
> > > > When this daemon is started after creating backend device, that device
> > > > will not be configured.
> > > >
> > > > Racy situation:
> > > > 1. driver domain is started
> > > > 2. frontend domain is started (just after kicking driver domain off)
> > > > 3. device in frontend domain is connected to the backend (as specified
> > > >in frontend domain configuration)
> > > > 4. xl devd is started in driver domain
> > > >
> > > > End result is that backend device in driver domain is not configured
> > > > (like network interface is not enabled), so the device doesn't work.
> > > >
> > > > Fix this by artifically triggering events for devices already present in
> > > > xenstore before xl devd is started. Do this only after xenstore watch is
> > > > already registered, and only for devices not already initialized (in
> > > > XenbusStateInitWait state).
> > >
> > > Thanks!
> > >
> > > > Cc: Ian Jackson
> > > > Cc: Wei Liu
> > > > Signed-off-by: Marek Marczykowski-Górecki
> > > >
> > > > ---
> > > > tools/libxl/libxl.c | 40
> > > > 1 file changed, 40 insertions(+)
> > > >
> > > > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> > > > index 1c81239..99815a7 100644
> > > > --- a/tools/libxl/libxl.c
> > > > +++ b/tools/libxl/libxl.c
> > > > @@ -4743,8 +4743,16 @@ int libxl_device_events_handler(libxl_ctx *ctx,
> > > > uint32_t domid;
> > > > libxl__ddomain ddomain;
> > > > char *be_path;
> > > > +char **kinds = NULL, **domains = NULL, **devs = NULL;
> > > > +const char *sstate;
> > > > +char *state_path;
> > > > +int state;
> > > > +unsigned int nkinds, ndomains, ndevs;
> > > > +int i, j, k;
> > > > +xs_transaction_t t;
> > > >
> > > > ddomain.ao = ao;
> > > > +FILLZERO(ddomain.watch);
> > >
> > > Is this a different bugfix or stray change?
> >
> > To cleanly unregister watch and not do nothing if wasn't registered at
> > all. If it isn't initialized, libxl__ev_xswatch_deregister call on
> > not registered watch isn't harmless.
>
> Right, I've realized that before your changes the function only registered
> the watch and exited, this is needed now.
>
> > > > LIBXL_SLIST_INIT(&ddomain.guests);
> > > >
> > > > rc = libxl__get_domid(gc, &domid);
> > > > @@ -4762,9 +4770,41 @@ int libxl_device_events_handler(libxl_ctx *ctx,
> > > > be_path);
> > > > if (rc) goto out;
> > > >
> > > > +rc = libxl__xs_transaction_start(gc, &t);
> > > > +if (rc) goto out;
> > >
> > > Why do you need to start a transaction here if you end up aborting it
> > > when
> > > finished?
> >
> > Mostly to ease error checking. Because below code does three level
> > listing, I don't want to deal with races where some entry was removed
> > between those calls, at least not here. Like this:
> >
> > xs_directory('backend/vif') -> 3, 4, 5
> > xs_directory('backend/vif/3') -> 0, 1
> > xs_read('backend/vif/3/0/state') -> ...
> > xs_read('backend/vif/3/1/state') -> ...
> > toolstack removes backend/vif/4 here
> > xs_directory('backend/vif/4') -> fail
> >
> > Of course backend_watch_callback would fail anyway in such a case, which
> > is ok. But having snapshot of xenstore during this multi-level listing
> > looks like avoiding some corner cases during listing itself.
>
> AFAICT your code seems to be prepared to deal with entries disappearing in
> the middle of the tree walk, so I would just remove the transaction.
Actually I'm considering changing error handling below to "goto out"
instead of "continue", as race condition should be eliminated by the
transaction, other errors (permission denied for example) maybe should
be considered fatal. So, IMO there two options:
- ignore failed reads and remove transaction
- error on failed reads and keep transaction
Which one would be better?
> > > > +kinds = libxl__xs_directory(gc, t, be_path, &nkinds);
> > > > +if (kinds) {
> > > > +for (i = 0; i < nkinds; i++) {
> > > > +domains = libxl__xs_directory(gc, t,
> > > > +GCSPRINTF("%s/%s", be_path, kinds[i]), &ndomains);
> > > > +if (!domains)
> > > > +continue;
> > > > +for (j = 0; j < ndomains; j++) {
> > > > +devs = libxl__xs_directory(gc, t,
> > > > +GCSPRINTF("%s/%s/%s", be_path, kinds[i],
> > > > domains[j]), &ndevs);
> > > > +if (!devs)
> > > > +continue;
> > > > +for (k = 0; k < ndevs; k++) {
> > > > +state_path = GCSPRINTF("%s/%s/%s/%s/state",
> > > > +
Re: [Xen-devel] [PATCHv3 1/2] libfs: allow simple_fill_super() to add symlinks
On 28/06/16 19:06, David Vrabel wrote:
> simple_fill_super() will add symlinks if an entry has mode & S_IFLNK.
> The target is provided in the new "link" field.
Can I get an ack for this, please? So it can go into 4.8 via the Xen tree.
David
>
> Signed-off-by: David Vrabel
> ---
> v2:
> - simplified.
> ---
> fs/libfs.c | 15 +--
> include/linux/fs.h | 2 +-
> 2 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/fs/libfs.c b/fs/libfs.c
> index cedeacb..bbf2d82 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -512,9 +512,20 @@ int simple_fill_super(struct super_block *s, unsigned
> long magic,
> dput(dentry);
> goto out;
> }
> - inode->i_mode = S_IFREG | files->mode;
> + if (files->mode & S_IFLNK) {
> + inode->i_mode = files->mode;
> + inode->i_op = &simple_symlink_inode_operations;
> + inode->i_link = kstrdup(files->link, GFP_KERNEL);
> + if (!inode->i_link) {
> + iput(inode);
> + dput(dentry);
> + goto out;
> + }
> + } else {
> + inode->i_mode = S_IFREG | files->mode;
> + inode->i_fop = files->ops;
> + }
> inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME;
> - inode->i_fop = files->ops;
> inode->i_ino = i;
> d_add(dentry, inode);
> }
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index dd28814..20c54a2 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2953,7 +2953,7 @@ extern const struct file_operations
> simple_dir_operations;
> extern const struct inode_operations simple_dir_inode_operations;
> extern void make_empty_dir_inode(struct inode *inode);
> extern bool is_empty_dir_inode(struct inode *inode);
> -struct tree_descr { char *name; const struct file_operations *ops; int mode;
> };
> +struct tree_descr { char *name; const struct file_operations *ops; int mode;
> char *link; };
> struct dentry *d_alloc_name(struct dentry *, const char *);
> extern int simple_fill_super(struct super_block *, unsigned long, struct
> tree_descr *);
> extern int simple_pin_fs(struct file_system_type *, struct vfsmount **mount,
> int *count);
>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl: trigger attach events for devices attached before xl devd startup
On Mon, Jul 11, 2016 at 10:56:04AM +0200, Marek Marczykowski-Górecki wrote:
> On Mon, Jul 11, 2016 at 10:31:17AM +0200, Roger Pau Monné wrote:
> > On Sun, Jul 10, 2016 at 07:35:47PM +0200, Marek Marczykowski-Górecki wrote:
> > > When this daemon is started after creating backend device, that device
> > > will not be configured.
> > >
> > > Racy situation:
> > > 1. driver domain is started
> > > 2. frontend domain is started (just after kicking driver domain off)
> > > 3. device in frontend domain is connected to the backend (as specified
> > >in frontend domain configuration)
> > > 4. xl devd is started in driver domain
> > >
> > > End result is that backend device in driver domain is not configured
> > > (like network interface is not enabled), so the device doesn't work.
> > >
> > > Fix this by artifically triggering events for devices already present in
> > > xenstore before xl devd is started. Do this only after xenstore watch is
> > > already registered, and only for devices not already initialized (in
> > > XenbusStateInitWait state).
> >
> > Thanks!
> >
> > > Cc: Ian Jackson
> > > Cc: Wei Liu
> > > Signed-off-by: Marek Marczykowski-Górecki
> > >
> > > ---
> > > tools/libxl/libxl.c | 40
> > > 1 file changed, 40 insertions(+)
> > >
> > > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> > > index 1c81239..99815a7 100644
> > > --- a/tools/libxl/libxl.c
> > > +++ b/tools/libxl/libxl.c
> > > @@ -4743,8 +4743,16 @@ int libxl_device_events_handler(libxl_ctx *ctx,
> > > uint32_t domid;
> > > libxl__ddomain ddomain;
> > > char *be_path;
> > > +char **kinds = NULL, **domains = NULL, **devs = NULL;
> > > +const char *sstate;
> > > +char *state_path;
> > > +int state;
> > > +unsigned int nkinds, ndomains, ndevs;
> > > +int i, j, k;
> > > +xs_transaction_t t;
> > >
> > > ddomain.ao = ao;
> > > +FILLZERO(ddomain.watch);
> >
> > Is this a different bugfix or stray change?
>
> To cleanly unregister watch and not do nothing if wasn't registered at
> all. If it isn't initialized, libxl__ev_xswatch_deregister call on
> not registered watch isn't harmless.
Right, I've realized that before your changes the function only registered
the watch and exited, this is needed now.
> > > LIBXL_SLIST_INIT(&ddomain.guests);
> > >
> > > rc = libxl__get_domid(gc, &domid);
> > > @@ -4762,9 +4770,41 @@ int libxl_device_events_handler(libxl_ctx *ctx,
> > > be_path);
> > > if (rc) goto out;
> > >
> > > +rc = libxl__xs_transaction_start(gc, &t);
> > > +if (rc) goto out;
> >
> > Why do you need to start a transaction here if you end up aborting it when
> > finished?
>
> Mostly to ease error checking. Because below code does three level
> listing, I don't want to deal with races where some entry was removed
> between those calls, at least not here. Like this:
>
> xs_directory('backend/vif') -> 3, 4, 5
> xs_directory('backend/vif/3') -> 0, 1
> xs_read('backend/vif/3/0/state') -> ...
> xs_read('backend/vif/3/1/state') -> ...
> toolstack removes backend/vif/4 here
> xs_directory('backend/vif/4') -> fail
>
> Of course backend_watch_callback would fail anyway in such a case, which
> is ok. But having snapshot of xenstore during this multi-level listing
> looks like avoiding some corner cases during listing itself.
AFAICT your code seems to be prepared to deal with entries disappearing in
the middle of the tree walk, so I would just remove the transaction.
> > > +kinds = libxl__xs_directory(gc, t, be_path, &nkinds);
> > > +if (kinds) {
> > > +for (i = 0; i < nkinds; i++) {
> > > +domains = libxl__xs_directory(gc, t,
> > > +GCSPRINTF("%s/%s", be_path, kinds[i]), &ndomains);
> > > +if (!domains)
> > > +continue;
> > > +for (j = 0; j < ndomains; j++) {
> > > +devs = libxl__xs_directory(gc, t,
> > > +GCSPRINTF("%s/%s/%s", be_path, kinds[i],
> > > domains[j]), &ndevs);
> > > +if (!devs)
> > > +continue;
> > > +for (k = 0; k < ndevs; k++) {
> > > +state_path = GCSPRINTF("%s/%s/%s/%s/state",
> > > +be_path, kinds[i], domains[j], devs[k]);
> > > +rc = libxl__xs_read_checked(gc, t, state_path,
> > > &sstate);
> > > +if (rc)
> > > +continue;
> > > +state = atoi(sstate);
> > > +if (state == XenbusStateInitWait)
> > > +backend_watch_callback(egc, &ddomain.watch,
> > > +be_path, state_path);
> > > +}
> > > +}
> > > +}
> > > +}
> > > +
> > > +libxl__xs_transaction_abort(gc, &t);
> > > +
> > > return AO_INPROGRESS;
> > >
> > > out:
>
Re: [Xen-devel] [PATCH] xen_pvscsi: reclaim the ring request when mapping data failed
On 11/07/16 04:51, Bin Wu wrote:
> During scsi command queueing, if mapping data fails, we need to
> reclaim the failed request. Otherwise, the garbage request will
> be pushed into the ring for the backend to work.
Well spotted. There is another instance of this problem in
scsifront_action_handler(). Would you mind correcting this one, too?
Juergen
>
> Signed-off-by: Bin Wu
> ---
> drivers/scsi/xen-scsifront.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/scsi/xen-scsifront.c b/drivers/scsi/xen-scsifront.c
> index 9dc8687..655163d 100644
> --- a/drivers/scsi/xen-scsifront.c
> +++ b/drivers/scsi/xen-scsifront.c
> @@ -565,6 +565,7 @@ static int scsifront_queuecommand(struct Scsi_Host
> *shost,
> err = map_data_for_request(info, sc, ring_req, shadow);
> if (err < 0) {
> pr_debug("%s: err %d\n", __func__, err);
> +info->ring.req_prod_pvt--;
> scsifront_put_rqid(info, rqid);
> scsifront_return(info);
> spin_unlock_irqrestore(shost->host_lock, flags);
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] Question about xen event channel
On 11/07/16 04:58, 소병철 wrote: > Hello everyone :) > > > > I have a question about xen event channel. Is it possible to allocate > two event channels in one xen frontend driver? Yes. For example, netif can have a separate Tx and Rx event channel. David ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] xen: arm64: Add support for Renesas RCar Gen3 H3 Salvator-X platform
Hi Dirk, On 08/07/16 12:38, Dirk Behme wrote: > Hi Andre, > > On 05.07.2016 16:29, Andre Przywara wrote: >> Hi, >> >> On 05/07/16 15:22, Dirk Behme wrote: >>> On 05.07.2016 15:45, Andre Przywara wrote: Hi, On 05/07/16 14:34, Julien Grall wrote: > (CC Andre) > > On 05/07/16 14:04, Dirk Behme wrote: >> On 05.07.2016 14:45, Julien Grall wrote: >>> >>> >>> On 05/07/16 13:09, Dirk Behme wrote: Hi Julien, On 05.07.2016 13:39, Julien Grall wrote: > Hi Dirk, > > On 05/07/16 07:37, Dirk Behme wrote: >> Signed-off-by: Dirk Behme > > This patch looks good to me, however I would like to see the > documentation on the wiki page (see [1]) before giving any formal > ack. Ok, many thanks for your review! Yes, I understood that something more is needed. I just wonder if we keep the patch on the mailing list for a moment. With this it's publically available and we can see how's the public interest for this board. Additionally, getting Xen running on this board and describe all this in the wiki isn't that easy. You either need to modify the flashed firmware. Or, like me, load everything via a JTAG debugger ... >>> >>> Can you details why you think it is not easy? Why do you have to >>> modify >>> the firmware? Is it because it does not boot the hypervisor in EL2? >> >> >> The board boots via ATF, which starts U-Boot in EL1, then. You >> have to >> find a way to load Xen into memory (e.g. U-Boot TFTP) and then >> switch to >> EL2 to start Xen. > > But ATF is running in EL3, right? If so, can ATF just starts U-boot > in EL2? > > I have a board at home (pine64) which also uses ATF and U-Boot and is > able to boot Xen without any SMC call because the U-Boot is entered > in EL2. From having a very quick look into the rcar ATF port on github[1] I see: +#elif (RCAR_BL33_EXECUTION_EL == BL33_EL2) +return (uint32_t)SPSR_64(MODE_EL2, MODE_SP_ELX, DISABLE_ALL_EXCEPTIONS); So if you rebuild ATF with RCAR_BL33_EXECUTION_EL set to BL33_EL2 that should enter U-Boot in EL2. The fix for the Pine64 was equally simple and works fine since then. >>> >>> >>> This is an other solution most probably I meant when talking about >>> "modifying the firmware" ;) >>> >>> I'm somehow unsure about the pros and cons running U-Boot at EL1 versus >>> EL2, though. >> >> It shouldn't matter, really. U-Boot on AArch64 (called armv8 here) is >> able to run in any exception level (in contrast to ARMv7). >> Actually running in EL2 is the architecturally recommended way, even >> with ATF (by default it drops into EL2 when returning to non-secure >> world). On Juno (and the Pine64) is works fine this way. >> >> As U-Boot only uses an identity mapping, many differences between EL2 >> and EL1 don't matter. > > An understanding question regarding this: We found that running U-Boot > at EL2 works fine and starting Xen works fine, then, too. So many thanks > for that hint! > > But we found that the Linux native boot case (without Xen) does fail, > then. I.e. running U-Boot in EL1 starts the native Linux kernel > successfully. While running U-Boot in EL2 starting the native Linux > kernel fails, then. That's rather odd, can you provide some debug information? > Any idea regarding this? > > Who is responsible to switch to EL1 for the native Linux boot case if > U-Boot runs at EL2? That's well supported kernel code in arch/arm64/kernel/head.S. If it sees the kernel to be entered in EL2 (which is highly recommended, btw), it installs the KVM HYP stub and drops into EL1 (after some register setup). You might want to check the code in there to see if something springs to mind or use some debug UART output to see where it gets stuck (if it gets stuck). I usually do something like "mov x1, #UART_TR; mov w0, #'1'; str w0, [x1]" to get an idea where low level code hangs. Cheers, Andre. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [libvirt test] 97003: tolerable FAIL - PUSHED
flight 97003 libvirt real [real]
http://logs.test-lab.xenproject.org/osstest/logs/97003/
Failures :-/ but no regressions.
Tests which did not succeed, but are not blocking:
test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass
test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail 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-i386-libvirt-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-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-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
version targeted for testing:
libvirt 32916b1699489584f66a53eaac322313803cabcc
baseline version:
libvirt 1edf20a9f87eb5873394182f470e191a9c70f9cf
Last test of basis96904 2016-07-10 04:22:13 Z1 days
Testing same since97003 2016-07-11 04:22:49 Z0 days1 attempts
People who touched revisions under test:
Fabian Freyer
Roman Bogorodskiy
jobs:
build-amd64-xsm pass
build-armhf-xsm pass
build-i386-xsm pass
build-amd64 pass
build-armhf pass
build-i386 pass
build-amd64-libvirt pass
build-armhf-libvirt pass
build-i386-libvirt pass
build-amd64-pvopspass
build-armhf-pvopspass
build-i386-pvops pass
test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm pass
test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass
test-amd64-amd64-libvirt-xsm pass
test-armhf-armhf-libvirt-xsm fail
test-amd64-i386-libvirt-xsm pass
test-amd64-amd64-libvirt pass
test-armhf-armhf-libvirt fail
test-amd64-i386-libvirt pass
test-amd64-amd64-libvirt-pairpass
test-amd64-i386-libvirt-pair pass
test-armhf-armhf-libvirt-qcow2 fail
test-armhf-armhf-libvirt-raw fail
test-amd64-amd64-libvirt-vhd pass
sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images
Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs
Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master
Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary
Pushing revision :
+ branch=libvirt
+ revision=32916b1699489584f66a53eaac322313803cabcc
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
getconfig Repos
perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x '!=' x/home/osstest/repos/lock ']'
++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock
++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push libvirt
32916b1699489584f66a53eaac322313803cabcc
+ branch=libvirt
+ revision=32916b1699489
Re: [Xen-devel] [PATCH] libxl: trigger attach events for devices attached before xl devd startup
On Mon, Jul 11, 2016 at 10:31:17AM +0200, Roger Pau Monné wrote:
> On Sun, Jul 10, 2016 at 07:35:47PM +0200, Marek Marczykowski-Górecki wrote:
> > When this daemon is started after creating backend device, that device
> > will not be configured.
> >
> > Racy situation:
> > 1. driver domain is started
> > 2. frontend domain is started (just after kicking driver domain off)
> > 3. device in frontend domain is connected to the backend (as specified
> >in frontend domain configuration)
> > 4. xl devd is started in driver domain
> >
> > End result is that backend device in driver domain is not configured
> > (like network interface is not enabled), so the device doesn't work.
> >
> > Fix this by artifically triggering events for devices already present in
> > xenstore before xl devd is started. Do this only after xenstore watch is
> > already registered, and only for devices not already initialized (in
> > XenbusStateInitWait state).
>
> Thanks!
>
> > Cc: Ian Jackson
> > Cc: Wei Liu
> > Signed-off-by: Marek Marczykowski-Górecki
> > ---
> > tools/libxl/libxl.c | 40
> > 1 file changed, 40 insertions(+)
> >
> > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> > index 1c81239..99815a7 100644
> > --- a/tools/libxl/libxl.c
> > +++ b/tools/libxl/libxl.c
> > @@ -4743,8 +4743,16 @@ int libxl_device_events_handler(libxl_ctx *ctx,
> > uint32_t domid;
> > libxl__ddomain ddomain;
> > char *be_path;
> > +char **kinds = NULL, **domains = NULL, **devs = NULL;
> > +const char *sstate;
> > +char *state_path;
> > +int state;
> > +unsigned int nkinds, ndomains, ndevs;
> > +int i, j, k;
> > +xs_transaction_t t;
> >
> > ddomain.ao = ao;
> > +FILLZERO(ddomain.watch);
>
> Is this a different bugfix or stray change?
To cleanly unregister watch and not do nothing if wasn't registered at
all. If it isn't initialized, libxl__ev_xswatch_deregister call on
not registered watch isn't harmless.
> > LIBXL_SLIST_INIT(&ddomain.guests);
> >
> > rc = libxl__get_domid(gc, &domid);
> > @@ -4762,9 +4770,41 @@ int libxl_device_events_handler(libxl_ctx *ctx,
> > be_path);
> > if (rc) goto out;
> >
> > +rc = libxl__xs_transaction_start(gc, &t);
> > +if (rc) goto out;
>
> Why do you need to start a transaction here if you end up aborting it when
> finished?
Mostly to ease error checking. Because below code does three level
listing, I don't want to deal with races where some entry was removed
between those calls, at least not here. Like this:
xs_directory('backend/vif') -> 3, 4, 5
xs_directory('backend/vif/3') -> 0, 1
xs_read('backend/vif/3/0/state') -> ...
xs_read('backend/vif/3/1/state') -> ...
toolstack removes backend/vif/4 here
xs_directory('backend/vif/4') -> fail
Of course backend_watch_callback would fail anyway in such a case, which
is ok. But having snapshot of xenstore during this multi-level listing
looks like avoiding some corner cases during listing itself.
> > +kinds = libxl__xs_directory(gc, t, be_path, &nkinds);
> > +if (kinds) {
> > +for (i = 0; i < nkinds; i++) {
> > +domains = libxl__xs_directory(gc, t,
> > +GCSPRINTF("%s/%s", be_path, kinds[i]), &ndomains);
> > +if (!domains)
> > +continue;
> > +for (j = 0; j < ndomains; j++) {
> > +devs = libxl__xs_directory(gc, t,
> > +GCSPRINTF("%s/%s/%s", be_path, kinds[i],
> > domains[j]), &ndevs);
> > +if (!devs)
> > +continue;
> > +for (k = 0; k < ndevs; k++) {
> > +state_path = GCSPRINTF("%s/%s/%s/%s/state",
> > +be_path, kinds[i], domains[j], devs[k]);
> > +rc = libxl__xs_read_checked(gc, t, state_path,
> > &sstate);
> > +if (rc)
> > +continue;
> > +state = atoi(sstate);
> > +if (state == XenbusStateInitWait)
> > +backend_watch_callback(egc, &ddomain.watch,
> > +be_path, state_path);
> > +}
> > +}
> > +}
> > +}
> > +
> > +libxl__xs_transaction_abort(gc, &t);
> > +
> > return AO_INPROGRESS;
> >
> > out:
> > +libxl__ev_xswatch_deregister(gc, &ddomain.watch);
>
> This seems to be part of a different bugfix also.
No, this code previously wasn't reachable if xswatch was correctly
registered.
--
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [BUG] kernel BUG at drivers/block/xen-blkfront.c:1711
On 06.06.2016 11:42, Dario Faggioli wrote:
Just Cc-ing some Linux, block, and Xen on CentOS people...
Ping.
Any suggestions how to debug this or what might cause the problem?
Obviously, we cannot control Xen on the Amazon's servers. But perhaps
there is something we can do at the kernel's side, is it?
On Mon, 2016-06-06 at 11:24 +0300, Evgenii Shatokhin wrote:
(Resending this bug report because the message I sent last week did
not
make it to the mailing list somehow.)
Hi,
One of our users gets kernel panics from time to time when he tries
to
use his Amazon EC2 instance with CentOS7 x64 in it [1]. Kernel panic
happens within minutes from the moment the instance starts. The
problem
does not show up every time, however.
The user first observed the problem with a custom kernel, but it was
found later that the stock kernel 3.10.0-327.18.2.el7.x86_64 from
CentOS7 was affected as well.
The part of the system log he was able to retrieve is attached. Here
is
the bug info, for convenience:
[2.246912] kernel BUG at drivers/block/xen-blkfront.c:1711!
[2.246912] invalid opcode: [#1] SMP
[2.246912] Modules linked in: ata_generic pata_acpi
crct10dif_pclmul
crct10dif_common crc32_pclmul crc32c_intel ghash_clmulni_intel
xen_netfront xen_blkfront(+) aesni_intel lrw ata_piix gf128mul
glue_helper ablk_helper cryptd libata serio_raw floppy sunrpc
dm_mirror
dm_region_hash dm_log dm_mod scsi_transport_iscsi
[2.246912] CPU: 1 PID: 50 Comm: xenwatch Not tainted
3.10.0-327.18.2.el7.x86_64 #1
[2.246912] Hardware name: Xen HVM domU, BIOS 4.2.amazon
12/07/2015
[2.246912] task: 8800e9fcb980 ti: 8800e98bc000 task.ti:
8800e98bc000
[2.246912] RIP: 0010:[] []
blkfront_setup_indirect+0x41f/0x430 [xen_blkfront]
[2.246912] RSP: 0018:8800e98bfcd0 EFLAGS: 00010283
[2.246912] RAX: 8800353e15c0 RBX: 8800e98c52c8 RCX:
0020
[2.246912] RDX: 8800353e15b0 RSI: 8800e98c52b8 RDI:
8800353e15d0
[2.246912] RBP: 8800e98bfd20 R08: 8800353e15b0 R09:
8800eb403c00
[2.246912] R10: a0155532 R11: ffe8 R12:
8800e98c4000
[2.246912] R13: 8800e98c52b8 R14: 0020 R15:
8800353e15c0
[2.246912] FS: () GS:8800efc2()
knlGS:
[2.246912] CS: 0010 DS: ES: CR0: 80050033
[2.246912] CR2: 7f1b615ef000 CR3: e2b44000 CR4:
001406e0
[2.246912] DR0: DR1: DR2:
[2.246912] DR3: DR6: 0ff0 DR7:
0400
[2.246912] Stack:
[2.246912] 0020 0001 0020a0157217
0100e98bfdbc
[2.246912] 27efa3ef 8800e98bfdbc 8800e98ce000
8800e98c4000
[2.246912] 8800e98ce040 0001 8800e98bfe08
a0155d4c
[2.246912] Call Trace:
[2.246912] [] blkback_changed+0x4ec/0xfc8
[xen_blkfront]
[2.246912] [] ? xenbus_gather+0x170/0x190
[2.246912] [] ? __slab_free+0x10e/0x277
[2.246912] []
xenbus_otherend_changed+0xad/0x110
[2.246912] [] ? xenwatch_thread+0x77/0x180
[2.246912] [] backend_changed+0x13/0x20
[2.246912] [] xenwatch_thread+0x66/0x180
[2.246912] [] ? wake_up_atomic_t+0x30/0x30
[2.246912] [] ?
unregister_xenbus_watch+0x1f0/0x1f0
[2.246912] [] kthread+0xcf/0xe0
[2.246912] [] ?
kthread_create_on_node+0x140/0x140
[2.246912] [] ret_from_fork+0x58/0x90
[2.246912] [] ?
kthread_create_on_node+0x140/0x140
[2.246912] Code: e1 48 85 c0 75 ce 49 8d 84 24 40 01 00 00 48 89
45
b8 e9 91 fd ff ff 4c 89 ff e8 8d ae 06 e1 e9 f2 fc ff ff 31 c0 e9 2e
fe
ff ff <0f> 0b e8 9a 57 f2 e0 0f 0b 0f 1f 84 00 00 00 00 00 0f 1f 44
00
[2.246912] RIP []
blkfront_setup_indirect+0x41f/0x430 [xen_blkfront]
[2.246912] RSP
[2.491574] ---[ end trace 8a9b992812627c71 ]---
[2.495618] Kernel panic - not syncing: Fatal exception
Xen version 4.2.
EC2 instance type: c3.large with EBS magnetic storage, if that
matters.
Here is the code where the BUG_ON triggers (drivers/block/xen-
blkfront.c):
if (!info->feature_persistent && info->max_indirect_segments) {
/*
* We are using indirect descriptors but not persistent
* grants, we need to allocate a set of pages that can be
* used for mapping indirect grefs
*/
int num = INDIRECT_GREFS(segs) * BLK_RING_SIZE;
BUG_ON(!list_empty(&info->indirect_pages)); // << This one hits.
for (i = 0; i < num; i++) {
struct page *indirect_page = alloc_page(GFP_NOIO);
if (!indirect_page)
goto out_of_memory;
list_add(&indirect_page->lru, &info->indirect_pages);
}
}
As we checked, 'info->indirect_pages' list indeed contained ar
Re: [Xen-devel] [PATCH] libxl: trigger attach events for devices attached before xl devd startup
On Sun, Jul 10, 2016 at 07:35:47PM +0200, Marek Marczykowski-Górecki wrote:
> When this daemon is started after creating backend device, that device
> will not be configured.
>
> Racy situation:
> 1. driver domain is started
> 2. frontend domain is started (just after kicking driver domain off)
> 3. device in frontend domain is connected to the backend (as specified
>in frontend domain configuration)
> 4. xl devd is started in driver domain
>
> End result is that backend device in driver domain is not configured
> (like network interface is not enabled), so the device doesn't work.
>
> Fix this by artifically triggering events for devices already present in
> xenstore before xl devd is started. Do this only after xenstore watch is
> already registered, and only for devices not already initialized (in
> XenbusStateInitWait state).
Thanks!
> Cc: Ian Jackson
> Cc: Wei Liu
> Signed-off-by: Marek Marczykowski-Górecki
> ---
> tools/libxl/libxl.c | 40
> 1 file changed, 40 insertions(+)
>
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 1c81239..99815a7 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -4743,8 +4743,16 @@ int libxl_device_events_handler(libxl_ctx *ctx,
> uint32_t domid;
> libxl__ddomain ddomain;
> char *be_path;
> +char **kinds = NULL, **domains = NULL, **devs = NULL;
> +const char *sstate;
> +char *state_path;
> +int state;
> +unsigned int nkinds, ndomains, ndevs;
> +int i, j, k;
> +xs_transaction_t t;
>
> ddomain.ao = ao;
> +FILLZERO(ddomain.watch);
Is this a different bugfix or stray change?
> LIBXL_SLIST_INIT(&ddomain.guests);
>
> rc = libxl__get_domid(gc, &domid);
> @@ -4762,9 +4770,41 @@ int libxl_device_events_handler(libxl_ctx *ctx,
> be_path);
> if (rc) goto out;
>
> +rc = libxl__xs_transaction_start(gc, &t);
> +if (rc) goto out;
Why do you need to start a transaction here if you end up aborting it when
finished?
> +kinds = libxl__xs_directory(gc, t, be_path, &nkinds);
> +if (kinds) {
> +for (i = 0; i < nkinds; i++) {
> +domains = libxl__xs_directory(gc, t,
> +GCSPRINTF("%s/%s", be_path, kinds[i]), &ndomains);
> +if (!domains)
> +continue;
> +for (j = 0; j < ndomains; j++) {
> +devs = libxl__xs_directory(gc, t,
> +GCSPRINTF("%s/%s/%s", be_path, kinds[i],
> domains[j]), &ndevs);
> +if (!devs)
> +continue;
> +for (k = 0; k < ndevs; k++) {
> +state_path = GCSPRINTF("%s/%s/%s/%s/state",
> +be_path, kinds[i], domains[j], devs[k]);
> +rc = libxl__xs_read_checked(gc, t, state_path, &sstate);
> +if (rc)
> +continue;
> +state = atoi(sstate);
> +if (state == XenbusStateInitWait)
> +backend_watch_callback(egc, &ddomain.watch,
> +be_path, state_path);
> +}
> +}
> +}
> +}
> +
> +libxl__xs_transaction_abort(gc, &t);
> +
> return AO_INPROGRESS;
>
> out:
> +libxl__ev_xswatch_deregister(gc, &ddomain.watch);
This seems to be part of a different bugfix also.
Roger.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
