Re: [Xen-devel] [RFC] xen/arm: Handling cache maintenance instructions by set/way
Hi, At 12:58 + on 06 Dec (1512565090), Julien Grall wrote: > On 12/06/2017 12:28 PM, George Dunlap wrote: > > 2. It sounds like rather than using PoD, you could use the > > "misconfigured p2m table" technique that x86 uses: set bits in the p2m > > entry which cause a specific kind of HAP fault when accessed. The fault > > handler then looks in the p2m entry, and if it finds an otherwise valid > > entry, it just fixes the "misconfigured" bits and continues. > > I thought about this. But when do you set the entry to misconfigured? > > If you take the example of Linux 32-bit. There are a couple of full > cache clean during the boot of uni-processor. So you would need to go > through the p2m multiple time and reset the access bits. My 2c (echoing what some others have already said): +1 for avoiding the full majesty of PoD if you don't need it. It should be possible to do something like the misconfigured-entry bit trick by _allocating_ the memory up-front and building the p2m entries but only making them usable by the {IO}MMUs on first access. That would make these early p2m walks shorter (because they can skip whole subtrees that aren't marked present yet) without making major changes to domain build or introducing run-time failures. Also beware of DoS conditions -- a guest that touches all its memory and then flushes by set/way mustn't be allowed to hurt the rest of the system. That probably means the set/way flush has to be preemptable. Tim. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC PATCH 06/31] cpufreq: make cpufreq driver more generalizable
>>> On 07.12.17 at 21:31, wrote: > Have questions which need to be clarified: > > If I understood correctly, new variant of set_px_pminfo is going to > have an extra "flag" argument, since > struct processor_performance doesn't have "flag" field (it contains > "state" field instead, which has yet another meaning). > Something like that: > int set_px_pminfo(uint32_t acpi_id, uint32_t flag, struct > processor_performance *dom0_px_info) > Is my understanding correct? Well, you obviously must not lose information, so having that extra parameter is unavoidable. Please use common sense when dealing with such re-structuring. And btw, please also be precise: There's no "flag" field, but there is a "flags" one. Such should also be the name of the new parameter - we're talking about multiple bits here, after all. > As for set_cx_pminfo(). To what struct we should do translation from > struct xen_processor_power? (struct acpi_processor_power?) Yes, of course. > Briefly looking at set_cx_pminfo(), I got a feeling, that in order to > modify it in a "set_px_pminfo() manner" > we need to rework print_cx_pminfo(), set_cx(), check_cx(), > acpi_processor_ffh_cstate_probe() too, since > all these function have arguments which contain XEN_GUEST_HANDLE. I am > wondering is it worth > doing such rework taking into the account that set_cx_pminfo() is not > going to be called from the non-hypercall context. > Or I missed something? Without looking at the details of this, please again use common sense. If there are good reasons for the two functions to not follow the same model, please simply state so in the overview mail of the patch series and/or (briefly, but concisely) in the specific patch's description. A good reason for example would be if overly large amounts of other code would need touching. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Ping: [PATCH] x86/mm: drop bogus assertion
Hi, At 02:31 -0700 on 06 Dec (1512527481), Jan Beulich wrote: > >>> On 30.11.17 at 10:10, wrote: > > i.e. the guest specified a runstate area address which has a non-present > > mapping in the page tables [EC=0002 CR2=88003d405220], but that's > > not something the hypervisor needs to be concerned about.) Release > > builds work fine, which is a first indication that the assertion isn't > > really needed. Yep, this assertion should just go away, so: Reviewed-by: Tim Deegan > > What's worse though - there appears to be a timing window where the > > guest runs in shadow mode, but not in log-dirty mode, and that is what > > triggers the assertion (the same could, afaict, be achieved by test- > > enabling shadow mode on a PV guest). This is because turing off log- > > dirty mode is being performed in two steps: First the log-dirty bit gets > > cleared (paging_log_dirty_disable() [having paused the domain] -> > > sh_disable_log_dirty() -> shadow_one_bit_disable()), followed by > > unpausing the domain and only then clearing shadow mode (via > > shadow_test_disable(), which pauses the domain a second time). > > > > Hence besides removing the ASSERT() here (or optionally replacing it by > > explicit translate and refcounts mode checks, but this seems rather > > pointless now that the three are tied together) I wonder whether either > > shadow_one_bit_disable() should turn off shadow mode if no other bit > > besides PG_SH_enable remains set (just like shadow_one_bit_enable() > > enables it if not already set), or the domain pausing scope should be > > extended so that both steps occur without the domain getting a chance to > > run in between. I'd be fine with either of those. It would avoid unexpected surprises in the relatively untested pv-shadow-without-logdirty paths. Tim. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 2/3] x86/acpi: take rsdp address for boot params if available
On 08/12/17 08:05, Ingo Molnar wrote: > > * Juergen Gross wrote: > >> In case the rsdp address in struct boot_params is specified don't try >> to find the table by searching, but take the address directly as set >> by the boot loader. >> >> Signed-off-by: Juergen Gross >> --- >> drivers/acpi/osl.c | 8 >> 1 file changed, 8 insertions(+) >> >> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c >> index 3bb46cb24a99..3b25e2ad7d75 100644 >> --- a/drivers/acpi/osl.c >> +++ b/drivers/acpi/osl.c >> @@ -45,6 +45,10 @@ >> #include >> #include >> >> +#ifdef CONFIG_X86 >> +#include >> +#endif >> + >> #include "internal.h" >> >> #define _COMPONENT ACPI_OS_SERVICES >> @@ -195,6 +199,10 @@ acpi_physical_address __init >> acpi_os_get_root_pointer(void) >> if (acpi_rsdp) >> return acpi_rsdp; >> #endif >> +#ifdef CONFIG_X86 >> +if (boot_params.hdr.acpi_rsdp_addr) >> +return boot_params.hdr.acpi_rsdp_addr; >> +#endif > > Argh, that's typical short sighted hackery, layering violations and general > eyesore combined into a single patch ... > > Those #ifdefs are a disgrace, plus why should generic ACPI code include > platform > details like boot_params.hdr/acpi_rsdp_addr? It's also not very extensible to > non-x86 - so someone will have to redo this work for ARM64 as well in the > future > ... > > So how about doing it right: > > 1) > > Add a __weak acpi_arch_get_root_pointer() __weak function to > drivers/acpi/osl.c: > > > __weak acpi_physical_address acpi_arch_get_root_pointer(void) > { > return 0; > } > > 2) > > use it in acpi_os_get_root_pointer(): > > ... > pa = acpi_arch_get_root_pointer(); > if (pa) > return pa; > ... > > 3) > > Override the default variant in x86's acpi.c via something like: > > acpi_physical_address acpi_arch_get_root_pointer(void) > { > return boot_params.hdr.acpi_rsdp_addr; > } > > 4) > > Add this to arch/x86/include/asm/acpi.h: > > extern acpi_physical_address acpi_arch_get_root_pointer(void); > > 5) > > Add #include to drivers/acpi/osl.c. > > > That looks much cleaner, has no layering violations and is infinitely more > extensible, right? Right. Thanks for the very constructive comment. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 1/3] x86/boot: add acpi rsdp address to setup_header
>>> On 08.12.17 at 08:16, wrote: > Also, a more fundamental question: why doesn't Xen use EFI to hand over > hardware configuration details? Iirc the main purpose of the change here is to allow booting PVH (guest or Dom0) with Grub2 in the middle. PVH, at least for the time being, is something that gets away without any firmware (and I'm pretty certain this is going to remain that way for Dom0). ACPI tables are being built by the tool stack (guest) or hypervisor (Dom0). Hence there simply isn't any EFI which could be used to propagate such information. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 1/3] x86/boot: add acpi rsdp address to setup_header
* Jan Beulich wrote: > >>> On 08.12.17 at 08:16, wrote: > > Also, a more fundamental question: why doesn't Xen use EFI to hand over > > hardware configuration details? > > Iirc the main purpose of the change here is to allow booting PVH > (guest or Dom0) with Grub2 in the middle. PVH, at least for the > time being, is something that gets away without any firmware > (and I'm pretty certain this is going to remain that way for Dom0). > ACPI tables are being built by the tool stack (guest) or hypervisor > (Dom0). Hence there simply isn't any EFI which could be used to > propagate such information. Ok, that's fair enough. If hpa (or someone else) doesn't object to the boot protocol extension this approach looks good to me in principle. Thanks, Ingo ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 1/3] x86/boot: add acpi rsdp address to setup_header
On 08/12/17 08:16, Ingo Molnar wrote: > > * Juergen Gross wrote: > >> Xen PVH guests receive the address of the RSDP table from Xen. In order >> to support booting a Xen PVH guest via grub2 using the standard x86 >> boot entry we need a way fro grub2 to pass the RSDP address to the >> kernel. >> >> For this purpose expand the struct setup_header to hold the physical >> address of the RSDP address. Being zero means it isn't specified and >> has to be located the legacy way (searching through low memory or >> EBDA). > > s/fro > /for > > pedantry: > > s/grub2 > /Grub2 Okay. > >> Signed-off-by: Juergen Gross >> Reviewed-by: Roger Pau Monné >> --- >> Documentation/x86/boot.txt| 19 +++ >> arch/x86/boot/header.S| 6 +- >> arch/x86/include/uapi/asm/bootparam.h | 1 + >> 3 files changed, 25 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/x86/boot.txt b/Documentation/x86/boot.txt >> index 5e9b826b5f62..a33c224797e4 100644 >> --- a/Documentation/x86/boot.txt >> +++ b/Documentation/x86/boot.txt >> @@ -61,6 +61,13 @@ Protocol 2.12:(Kernel 3.8) Added the xloadflags field >> and extension fields >> to struct boot_params for loading bzImage and ramdisk >> above 4G in 64bit. >> >> +Protocol 2.13: (Kernel 3.14) Support 32- and 64-bit flags being set in >> +xloadflags to support booting a 64 bit kernel from 32 bit >> +EFI > > The changelog should I think declare that we add documentation for the 2.13 > protocol iteration as well. > > Also, please use a consistent spelling of '32-bit' and '64-bit' in the same > sentence! Okay. > >> +Field name: acpi_rsdp_addr >> +Type: write >> +Offset/size:0x268/8 >> +Protocol: 2.14+ >> + >> + This field can be set by the boot loader to tell the kernel the >> + physical address of the ACPI RSDP table. >> + >> + A value of 0 indicates the kernel should fall back to the standard >> + methods to locate the RSDP (search in EBDA/low memory). > > That's not the only method used: the ACPI RSDP address can also be discovered > via > efi.rsdp20 and efi.rsdp, both of which appear to be 32-bit values. Sure, but this is valid for booting via EFI only. > >> THE IMAGE CHECKSUM >> >> diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S >> index 850b8762e889..e7184127f309 100644 >> --- a/arch/x86/boot/header.S >> +++ b/arch/x86/boot/header.S >> @@ -300,7 +300,7 @@ _start: >> # Part 2 of the header, from the old setup.S >> >> .ascii "HdrS" # header signature >> -.word 0x020d # header version number (>= 0x0105) >> +.word 0x020e # header version number (>= 0x0105) >> # or else old loadlin-1.5 will fail) >> .globl realmode_swtch >> realmode_swtch: .word 0, 0# default_switch, SETUPSEG >> @@ -558,6 +558,10 @@ pref_address: .quad LOAD_PHYSICAL_ADDR >> # preferred load addr >> init_size: .long INIT_SIZE # kernel initialization size >> handover_offset:.long 0 # Filled in by build.c >> >> +acpi_rsdp_addr: .quad 0 # 64-bit physical >> pointer to >> +# ACPI RSDP table, added with >> +# version 2.14 > > s/pointer to ACPI RSDP table > /pointer to the ACPI RSDP table Okay. > > Also, a more fundamental question: why doesn't Xen use EFI to hand over > hardware > configuration details? I think Jan has answered this question quite well. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 3/3] x86/xen: supply rsdp address in boot params for pvh guests
On 08/12/17 08:22, Ingo Molnar wrote: > > * Juergen Gross wrote: > >> When booted via the special PVH entry save the RSDP address set in the >> boot information block in struct boot_params. This will enable Xen to >> locate the RSDP at an arbitrary address. >> >> Set the boot loader version to 2.14 (0x020e) replacing the wrong 0x0212 >> which should have been 0x020c. >> >> Signed-off-by: Juergen Gross >> --- >> V2: set bootloader version to 2.14 (Roger Pau Monné) >> --- >> arch/x86/xen/enlighten_pvh.c | 5 - >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/arch/x86/xen/enlighten_pvh.c b/arch/x86/xen/enlighten_pvh.c >> index 436c4f003e17..036e3a5f284a 100644 >> --- a/arch/x86/xen/enlighten_pvh.c >> +++ b/arch/x86/xen/enlighten_pvh.c >> @@ -68,9 +68,12 @@ static void __init init_pvh_bootparams(void) >> * >> * Version 2.12 supports Xen entry point but we will use default x86/PC >> * environment (i.e. hardware_subarch 0). >> + * The RSDP address is available from version 2.14 on. >> */ >> -pvh_bootparams.hdr.version = 0x212; >> +pvh_bootparams.hdr.version = 0x20e; > > While 0x212 was "obvious" to read but totally wrong, it would be less fragile > and > more readable if the version was generated as something like: > > pvh_bootparams.hdr.version = (2 << 8) | 14; Sure, I can make that change. > > similar to how it's written in other cases: > >> pvh_bootparams.hdr.type_of_loader = (9 << 4) | 0; /* Xen loader */ > > Also, shouldn't the 0x212 fix be a separate patch, Cc: stable? The bug > appears to > have been introduced at around v4.12. While not really being very important, this seems to be cleaner, yes. After all this value is visible in sysfs, so it should be correct. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 1/3] x86/boot: add acpi rsdp address to setup_header
* Juergen Gross wrote: > >> +Offset/size: 0x268/8 > >> +Protocol: 2.14+ > >> + > >> + This field can be set by the boot loader to tell the kernel the > >> + physical address of the ACPI RSDP table. > >> + > >> + A value of 0 indicates the kernel should fall back to the standard > >> + methods to locate the RSDP (search in EBDA/low memory). > > > > That's not the only method used: the ACPI RSDP address can also be > > discovered via > > efi.rsdp20 and efi.rsdp, both of which appear to be 32-bit values. > > Sure, but this is valid for booting via EFI only. Yeah, so what I tried to say is that the description as written is not fully correct and triggered my pedantry: + A value of 0 indicates the kernel should fall back to the standard + methods to locate the RSDP (search in EBDA/low memory). To make it correct we need to either write less: + A value of 0 indicates the kernel should fall back to the standard + methods to locate the RSDP. or write more and make it open ended so it doesn't have to be extended with every method of getting the RSDP that might be added in the future: + A value of 0 indicates the kernel should fall back to the standard + methods to locate the RSDP (search in EBDA/low memory, get it from + EFI if present, etc.). ... or so? Thanks, Ingo ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC PATCH v2 1/2] xen/pvh: Add memory map pointer to hvm_start_info struct
>>> On 07.12.17 at 23:45, wrote: > The start info structure that is defined as part of the x86/HVM direct > boot ABI and used for starting Xen PVH guests would be more versatile if > it also included a way to efficiently pass information about the memory > map to the guest. > > That way Xen PVH guests would not be forced to use a hypercall to get the > information and would make it easier for KVM guests to share the PVH > entry point. > --- > include/xen/interface/hvm/start_info.h | 34 > +++--- > 1 file changed, 31 insertions(+), 3 deletions(-) First of all such a change should be submitted against the canonical copy of the header, which lives in the Xen tree. The argument of avoiding a hypercall doesn't really count imo - this isn't in any way performance critical code. The argument of making re-use easier is fine, though. > --- a/include/xen/interface/hvm/start_info.h > +++ b/include/xen/interface/hvm/start_info.h > @@ -33,7 +33,7 @@ > *| magic | Contains the magic value XEN_HVM_START_MAGIC_VALUE > *|| ("xEn3" with the 0x80 bit of the "E" set). > * 4 ++ > - *| version| Version of this structure. Current version is 0. New > + *| version| Version of this structure. Current version is 1. New > *|| versions are guaranteed to be backwards-compatible. > * 8 ++ > *| flags | SIF_xxx flags. > @@ -48,6 +48,12 @@ > * 32 ++ > *| rsdp_paddr | Physical address of the RSDP ACPI data structure. > * 40 ++ > + *| memmap_paddr | Physical address of the memory map. Only present in > + *|| version 1 and newer of the structure. > + * 48 ++ > + *| memmap_entries | Number of entries in the memory map table. Only > + *|| present in version 1 and newer of the structure. > + * 52 ++ Please let's make this optional even in v1 (and later), i.e. spell out that it may be zero. That way Xen code could continue to use the hypercall approach even. Also please spell out a 4-byte reserved entry at the end, to make the specified structure a multiple of 8 in size again regardless of bitness of the producer/consumer. > @@ -62,6 +68,17 @@ > *| reserved | > * 32 ++ > * > + * The layout of each entry in the memory map table is as follows and no > + * padding is used between entries in the array: > + * > + * 0 ++ > + *| addr | Base address > + * 8 ++ > + *| size | Size of mapping > + * 16 ++ > + *| type | E820_TYPE_xxx > + * 20 +| I'm not convinced of re-using E820 types here. I can see that this might ease the consumption in Linux, but I don't think there should be any connection to x86 aspects here - the data being supplied is x86-agnostic, and Linux'es placement of the header is also making no connection to x86 (oddly enough, the current placement in the Xen tree does, for a reason which escapes me). I could also imagine reasons to add new types without them being sanctioned by whoever maintains E820 type assignments. As to the size field - you need to spell out whether these are bytes or pages (it might be worthwhile to also make this explicit for the addr one, but there I view it as less of a problem, since "address" doesn't commonly mean a page granular entity). Also this again lacks a 4-byte reserved field at the end. > @@ -86,13 +103,24 @@ struct hvm_start_info { > uint64_t cmdline_paddr; /* Physical address of the command line. > */ > uint64_t rsdp_paddr;/* Physical address of the RSDP ACPI data > */ > /* structure. > */ > -}; > +uint64_t memmap_paddr; /* Physical address of an array of */ > + /* hvm_memmap_table_entry. Only present in */ > + /* Ver 1 or later. For e820 mem map table. */ > +uint32_t memmap_entries; /* Only present in Ver 1 or later. Number of */ > + /* entries in the memmap table. */ > +} __attribute__((packed)); No packed attribute here and below please, at least not in the canonical (non-Linux) variant of the header. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 1/3] x86/boot: add acpi rsdp address to setup_header
On 08/12/17 09:48, Ingo Molnar wrote: > > * Juergen Gross wrote: > +Offset/size: 0x268/8 +Protocol: 2.14+ + + This field can be set by the boot loader to tell the kernel the + physical address of the ACPI RSDP table. + + A value of 0 indicates the kernel should fall back to the standard + methods to locate the RSDP (search in EBDA/low memory). >>> >>> That's not the only method used: the ACPI RSDP address can also be >>> discovered via >>> efi.rsdp20 and efi.rsdp, both of which appear to be 32-bit values. >> >> Sure, but this is valid for booting via EFI only. > > Yeah, so what I tried to say is that the description as written is not fully > correct and triggered my pedantry: > > + A value of 0 indicates the kernel should fall back to the standard > + methods to locate the RSDP (search in EBDA/low memory). > > To make it correct we need to either write less: > > + A value of 0 indicates the kernel should fall back to the standard > + methods to locate the RSDP. > > or write more and make it open ended so it doesn't have to be extended with > every > method of getting the RSDP that might be added in the future: > > + A value of 0 indicates the kernel should fall back to the standard > + methods to locate the RSDP (search in EBDA/low memory, get it from > + EFI if present, etc.). > > ... or so? Aah, okay. I got your remark wrong then. I think I'll go with the shorter variant. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [GIT PULL] xen: fixes for 4.15-rc3
Linus, Please git pull the following tag: git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git for-linus-4.15-rc3-tag xen: fixes for 4.15-rc3 Those are just two small fixes for the nev pvcalls frontend driver. Thanks. Juergen drivers/xen/pvcalls-front.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) Dan Carpenter (2): xen/pvcalls: check for xenbus_read() errors xen/pvcalls: Fix a check in pvcalls_front_remove() ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH V3 2/2] Xen/PCIback: Implement PCI flr/slot/bus reset with 'reset' SysFS attribute
>>> On 07.12.17 at 23:21, wrote: > Due to the complexity with the PCI lock we cannot do the reset when a > device is bound ('echo $BDF > bind') or when unbound ('echo $BDF > unbind') > as the pci_[slot|bus]_reset also takes the same lock resulting in a > dead-lock. It took me a moment to figure that here you're referring to the process of (un)binding, not the state. To avoid that ambiguity in wording, how about "... we cannot do the reset while a device is being bound (...) or while it is being unbound ..."? > --- a/Documentation/ABI/testing/sysfs-driver-pciback > +++ b/Documentation/ABI/testing/sysfs-driver-pciback > @@ -11,3 +11,18 @@ Description: > #echo 00:19.0-E0:2:FF > /sys/bus/pci/drivers/pciback/quirks > will allow the guest to read and write to the configuration > register 0x0E. > + > +What: /sys/bus/pci/drivers/pciback/reset > +Date: Dec 2017 > +KernelVersion: 4.15 > +Contact:xen-devel@lists.xenproject.org > +Description: > +An option to perform a flr/slot/bus reset when a PCI device > + is owned by Xen PCI backend. Writing a string of :BB:DD.F :BB:DD.F (or else the D-s are ambiguous, the more that "domain" in Xen code is ambiguous anyway - I continue to be mislead by struct pcistub_device_id's domain field) Also I assume the part is optional (default zero), which probably can and should be expressed in some way. > --- a/drivers/xen/xen-pciback/pci_stub.c > +++ b/drivers/xen/xen-pciback/pci_stub.c > @@ -313,6 +313,102 @@ void pcistub_put_pci_dev(struct pci_dev *dev) > up_write(&pcistub_sem); > } > > +struct pcistub_args { > + const struct pci_dev *dev; > + unsigned int dcount; The sole use of this field is for a debug message. Why not drop it and make "dev" the "data" argument without further indirection? > +static int pcistub_device_search(struct pci_dev *dev, void *data) > +{ > + struct pcistub_device *psdev; > + struct pcistub_args *arg = data; > + bool found = false; > + unsigned long flags; > + > + spin_lock_irqsave(&pcistub_devices_lock, flags); > + > + list_for_each_entry(psdev, &pcistub_devices, dev_list) { > + if (psdev->dev == dev) { > + found = true; > + arg->dcount++; > + break; Neither here nor in the caller I can see a check whether the device is currently assigned to a guest. Ownership by pciback alone imo is not sufficient to allow a reset to be performed. > +static int pcistub_device_reset(struct pci_dev *dev) > +{ > + struct xen_pcibk_dev_data *dev_data; > + bool slot = false, bus = false; > + struct pcistub_args arg = {}; > + > + if (!dev) > + return -EINVAL; > + > + dev_dbg(&dev->dev, "[%s]\n", __func__); > + > + /* First check and try FLR */ > + if (pcie_has_flr(dev)) { > + dev_dbg(&dev->dev, "resetting %s device using FLR\n", > + pci_name(dev)); > + pcie_flr(dev); The lack of error check here puzzled me, but I see the function indeed returns void right now. I think the prereq patch should change this along with exporting the function - you really don't want the device to be handed to a guest when the FLR timed out. > + return 0; > + } > + > + if (!pci_probe_reset_slot(dev->slot)) > + slot = true; > + else if ((!pci_probe_reset_bus(dev->bus)) && > + (!pci_is_root_bus(dev->bus))) Too many parentheses for my taste. > +static ssize_t reset_store(struct device_driver *drv, const char *buf, > +size_t count) > +{ > + struct pcistub_device *psdev; > + int domain, bus, slot, func; > + int err; > + > + err = str_to_slot(buf, &domain, &bus, &slot, &func); > + if (err) > + return err; > + > + psdev = pcistub_device_find(domain, bus, slot, func); > + if (psdev) { > + err = pcistub_device_reset(psdev->dev); > + pcistub_device_put(psdev); > + } else { > + err = -ENODEV; > + } > + > + if (!err) > + err = count; > + > + return err; > +} > +static DRIVER_ATTR_WO(reset); Would it be worth for reads of the file to return whether the device can be reset this way (i.e. the result of the checks you do before actually doing the reset)? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [distros-debian-jessie test] 72526: tolerable all pass
flight 72526 distros-debian-jessie real [real] http://osstest.xs.citrite.net/~osstest/testlogs/logs/72526/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-armhf-armhf-armhf-jessie-netboot-pygrub 12 migrate-support-check fail like 72505 test-armhf-armhf-armhf-jessie-netboot-pygrub 13 saverestore-support-check fail like 72505 baseline version: flight 72505 jobs: build-amd64 pass build-armhf pass build-i386 pass build-amd64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-amd64-jessie-netboot-pvgrub pass test-amd64-i386-i386-jessie-netboot-pvgrub pass test-amd64-i386-amd64-jessie-netboot-pygrub pass test-armhf-armhf-armhf-jessie-netboot-pygrub pass test-amd64-amd64-i386-jessie-netboot-pygrub pass sg-report-flight on osstest.xs.citrite.net logs: /home/osstest/logs images: /home/osstest/images Logs, config files, etc. are available at http://osstest.xs.citrite.net/~osstest/testlogs/logs Test harness code can be found at http://xenbits.xensource.com/gitweb?p=osstest.git;a=summary Push not applicable. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 22/25] x86/HVM: do actual CMPXCHG in hvmemul_cmpxchg()
> -Original Message- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: 07 December 2017 14:17 > To: xen-devel > Cc: Andrew Cooper ; Paul Durrant > ; George Dunlap > Subject: [PATCH v3 22/25] x86/HVM: do actual CMPXCHG in > hvmemul_cmpxchg() > > ..., at least as far as currently possible, i.e. when a mapping can be > obtained. > > Signed-off-by: Jan Beulich Reviewed-by: Paul Durrant > --- > v3: New. > > --- a/xen/arch/x86/hvm/emulate.c > +++ b/xen/arch/x86/hvm/emulate.c > @@ -1296,8 +1296,83 @@ static int hvmemul_cmpxchg( > bool lock, > struct x86_emulate_ctxt *ctxt) > { > -/* Fix this in case the guest is really relying on r-m-w atomicity. */ > -return hvmemul_write(seg, offset, p_new, bytes, ctxt); > +struct hvm_emulate_ctxt *hvmemul_ctxt = > +container_of(ctxt, struct hvm_emulate_ctxt, ctxt); > +struct vcpu *curr = current; > +unsigned long addr, reps = 1; > +uint32_t pfec = PFEC_page_present | PFEC_write_access; > +struct hvm_vcpu_io *vio = &curr->arch.hvm_vcpu.hvm_io; > +int rc; > +void *mapping = NULL; > + > +rc = hvmemul_virtual_to_linear( > +seg, offset, bytes, &reps, hvm_access_write, hvmemul_ctxt, &addr); > +if ( rc != X86EMUL_OKAY ) > +return rc; > + > +if ( is_x86_system_segment(seg) ) > +pfec |= PFEC_implicit; > +else if ( hvmemul_ctxt->seg_reg[x86_seg_ss].dpl == 3 ) > +pfec |= PFEC_user_mode; > + > +mapping = hvmemul_map_linear_addr(addr, bytes, pfec, > hvmemul_ctxt); > +if ( IS_ERR(mapping) ) > +return ~PTR_ERR(mapping); > + > +if ( !mapping ) > +{ > +/* Fix this in case the guest is really relying on r-m-w atomicity. > */ > +return hvmemul_linear_mmio_write(addr, bytes, p_new, pfec, > + hvmemul_ctxt, > + vio->mmio_access.write_access && > + vio->mmio_gla == (addr & > PAGE_MASK)); > +} > + > +switch ( bytes ) > +{ > +case 1: case 2: case 4: case 8: > +{ > +unsigned long old = 0, new = 0, cur; > + > +memcpy(&old, p_old, bytes); > +memcpy(&new, p_new, bytes); > +if ( lock ) > +cur = __cmpxchg(mapping, old, new, bytes); > +else > +cur = cmpxchg_local_(mapping, old, new, bytes); > +if ( cur != old ) > +{ > +memcpy(p_old, &cur, bytes); > +rc = X86EMUL_CMPXCHG_FAILED; > +} > +break; > +} > + > +case 16: > +if ( cpu_has_cx16 ) > +{ > +__uint128_t *old = p_old, cur; > + > +if ( lock ) > +cur = __cmpxchg16b(mapping, old, p_new); > +else > +cur = cmpxchg16b_local_(mapping, old, p_new); > +if ( cur != *old ) > +{ > +*old = cur; > +rc = X86EMUL_CMPXCHG_FAILED; > +} > +break; > +} > +/* fall through */ > +default: > +rc = X86EMUL_UNHANDLEABLE; > +break; > +} > + > +hvmemul_unmap_linear_addr(mapping, addr, bytes, hvmemul_ctxt); > + > +return rc; > } > > static int hvmemul_validate( > --- a/xen/include/asm-x86/system.h > +++ b/xen/include/asm-x86/system.h > @@ -110,6 +110,38 @@ static always_inline unsigned long __cmp > return old; > } > > +static always_inline unsigned long cmpxchg_local_( > +void *ptr, unsigned long old, unsigned long new, unsigned int size) > +{ > +unsigned long prev = ~old; > + > +switch ( size ) > +{ > +case 1: > +asm volatile ( "cmpxchgb %b2, %1" > + : "=a" (prev), "+m" (*(uint8_t *)ptr) > + : "q" (new), "0" (old) ); > +break; > +case 2: > +asm volatile ( "cmpxchgw %w2, %1" > + : "=a" (prev), "+m" (*(uint16_t *)ptr) > + : "r" (new), "0" (old) ); > +break; > +case 4: > +asm volatile ( "cmpxchgl %k2, %1" > + : "=a" (prev), "+m" (*(uint32_t *)ptr) > + : "r" (new), "0" (old) ); > +break; > +case 8: > +asm volatile ( "cmpxchgq %2, %1" > + : "=a" (prev), "+m" (*(uint64_t *)ptr) > + : "r" (new), "0" (old) ); > +break; > +} > + > +return prev; > +} > + > #define cmpxchgptr(ptr,o,n) ({ \ > const __typeof__(**(ptr)) *__o = (o); \ > __typeof__(**(ptr)) *__n = (n); \ > --- a/xen/include/asm-x86/x86_64/system.h > +++ b/xen/include/asm-x86/x86_64/system.h > @@ -31,6 +31,24 @@ static always_inline __uint128_t __cmpxc > return prev.raw; > } > > +static always_inline __uint128_t cmpxchg16b_local_( > +void *ptr, const __uint128_t *oldp, const __uint128_t *n
Re: [Xen-devel] [PATCH v3 23/25] x86/HVM: make use of new read-modify-write emulator hook
> -Original Message- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: 07 December 2017 14:18 > To: xen-devel > Cc: Andrew Cooper ; Paul Durrant > ; George Dunlap > Subject: [PATCH v3 23/25] x86/HVM: make use of new read-modify-write > emulator hook > > ..., at least as far as currently possible, i.e. when a mapping can be > obtained. > > Signed-off-by: Jan Beulich Reviewed-by: Paul Durrant > --- > v3: New. > > --- a/xen/arch/x86/hvm/emulate.c > +++ b/xen/arch/x86/hvm/emulate.c > @@ -1187,6 +1187,61 @@ static int hvmemul_write( > return X86EMUL_OKAY; > } > > +static int hvmemul_rmw( > +enum x86_segment seg, > +unsigned long offset, > +unsigned int bytes, > +uint32_t *eflags, > +struct x86_emulate_state *state, > +struct x86_emulate_ctxt *ctxt) > +{ > +struct hvm_emulate_ctxt *hvmemul_ctxt = > +container_of(ctxt, struct hvm_emulate_ctxt, ctxt); > +unsigned long addr, reps = 1; > +uint32_t pfec = PFEC_page_present | PFEC_write_access; > +struct hvm_vcpu_io *vio = ¤t->arch.hvm_vcpu.hvm_io; > +int rc; > +void *mapping; > + > +rc = hvmemul_virtual_to_linear( > +seg, offset, bytes, &reps, hvm_access_write, hvmemul_ctxt, &addr); > +if ( rc != X86EMUL_OKAY || !bytes ) > +return rc; > + > +if ( is_x86_system_segment(seg) ) > +pfec |= PFEC_implicit; > +else if ( hvmemul_ctxt->seg_reg[x86_seg_ss].dpl == 3 ) > +pfec |= PFEC_user_mode; > + > +mapping = hvmemul_map_linear_addr(addr, bytes, pfec, > hvmemul_ctxt); > +if ( IS_ERR(mapping) ) > +return ~PTR_ERR(mapping); > + > +if ( mapping ) > +{ > +rc = x86_emul_rmw(mapping, bytes, eflags, state, ctxt); > +hvmemul_unmap_linear_addr(mapping, addr, bytes, hvmemul_ctxt); > +} > +else > +{ > +unsigned long data = 0; > +bool_t known_gpfn = vio->mmio_access.write_access && > +vio->mmio_gla == (addr & PAGE_MASK); > + > +if ( bytes > sizeof(data) ) > +return X86EMUL_UNHANDLEABLE; > +rc = hvmemul_linear_mmio_read(addr, bytes, &data, pfec, > hvmemul_ctxt, > + known_gpfn); > +if ( rc == X86EMUL_OKAY ) > +rc = x86_emul_rmw(&data, bytes, eflags, state, ctxt); > +if ( rc == X86EMUL_OKAY ) > +rc = hvmemul_linear_mmio_write(addr, bytes, &data, pfec, > + hvmemul_ctxt, known_gpfn); > +} > + > +return rc; > +} > + > static int hvmemul_write_discard( > enum x86_segment seg, > unsigned long offset, > @@ -2157,6 +2212,7 @@ static const struct x86_emulate_ops hvm_ > .read = hvmemul_read, > .insn_fetch= hvmemul_insn_fetch, > .write = hvmemul_write, > +.rmw = hvmemul_rmw, > .cmpxchg = hvmemul_cmpxchg, > .validate = hvmemul_validate, > .rep_ins = hvmemul_rep_ins, > > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-4.1 test] 116949: regressions - FAIL
flight 116949 linux-4.1 real [real] http://logs.test-lab.xenproject.org/osstest/logs/116949/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl-qemut-win7-amd64 10 windows-install fail REGR. vs. 116145 test-amd64-amd64-xl-qemut-win7-amd64 10 windows-install fail REGR. vs. 116145 Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 116145 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail like 116145 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 116145 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 116145 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail like 116145 test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-xl-pvhv2-amd 12 guest-start fail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-qcow2 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-armhf-armhf-xl-xsm 13 migrate-support-checkfail never pass test-armhf-armhf-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass version targeted for testing: linuxc3f4bb14a264a96c2709b026dca37a4eb252a82a baseline version: linux200d858d94b4d8ed7a287e3a3c2b860ae9e17e83 Last test of basis 116145 2017-11-13 18:11:06 Z 24 days Testing same since 116949 2017-12-07 17:33:09 Z0 days1 attempts People who touched revisions under test: "Eric W. Biederman" "HUANG Weller (CM/ESW12-CN)" Aaron Brown Aaron Sierra Adam Wallis Akinobu Mita Al Viro Alan Stern Alex Chen Alexander Boyko Alexander Duyck Alexander Potapenko Alexei Starovoitov Alexey Khoroshilov Alison Schofield Andrei Vagin Andrew Bowers Andrew Morton Andrey Konovalov Andrey Ryabinin Andrii Anisov Andy Shevchenko Arnd Bergmann Arvind Yadav Ashish Samant Bart Van Assche Bartlomiej Zolnierkiewicz Bernhard Rosenkraenzer Bilal Amarni Bjorn Helgaas Bjørn Mork Boris Ostrovsky Borislav Petkov Brian Norris Carlo Caione Catalin Marinas Changwei Ge Chanwoo Choi Chi-hsien
Re: [Xen-devel] [RFC] xen/arm: Handling cache maintenance instructions by set/way
On 12/07/2017 07:21 PM, Marc Zyngier wrote: > On 07/12/17 18:06, George Dunlap wrote: >> On 12/07/2017 04:58 PM, Marc Zyngier wrote: >>> On 07/12/17 16:44, George Dunlap wrote: On 12/07/2017 04:04 PM, Julien Grall wrote: > Hi Jan, > > On 07/12/17 15:45, Jan Beulich wrote: > On 07.12.17 at 15:53, wrote: >>> On 07/12/17 13:52, Julien Grall wrote: >>> There is exactly one case where set/way makes sense, and that's when >>> you're the only CPU left in the system, your MMU is off, and you're >>> about to go down. >> >> With this and ... >> >>> On top of bypassing the coherency, S/W CMOs do not prevent lines from >>> migrating from one CPU to another. So you could happily be flushing by >>> S/W, and still end up with dirty lines in your cache. Success! >> >> ... this I wonder what value emulating those insns then has in the first >> place. Can't you as well simply skip and ignore them, with the same >> (bad) result? > > The result will be much much worst. Here a concrete example with a Linux > Arm 32-bit: > > 1) Cache enabled > 2) Decompress > 3) Nuke cache (S/W) > 4) Cache off > 5) Access new kernel > > If you skip #3, the decompress data may not have reached the memory, so > you would access stall data. > > This would effectively mean we don't support Linux Arm 32-bit. So Marc said that #3 "doesn't make sense", since although it might be the only cpu on in the system, you're not "about to go down"; but Linux 32-bit is doing that anyway. >>> >>> "Doesn't make sense" on an ARMv7+ with SMP. That code dates back to >>> ARMv4, and has been left untouched ever since. "If it ain't broke..." >>> It sounds like from the slides the purpose of #3 might be to get stuff out of the D-cache into the I-cache. But why is the cache turned off? >>> >>> Linux mandates that the kernel in entered with the MMU off. Which has >>> the effect of disabling the caches too (VIVT caches and all that jazz). >>> And why doesn't Linux use the VA-based flushes rather than the S/W flushes? >>> >>> Linux/arm64 does. Changing the 32bit port to use VA CMOs would probably >>> break stuff from the late 90s, so that's not going to happen. These >>> days, I tend to pick my battles... ;-) >> >> OK, so let me try to state this "forwards" for those of us not familiar >> with the situation: >> >> 1. Linux expects to start in 'linear' mode, with the MMU disabled. >> >> 2. On ARM, disabling the MMU disables caching (!). But disabling >> caching doesn't flush the cache; it just means the cache is bypassed (!). >> >> 3. Which means for Linux on ARM, after unzipping the kernel image, you >> need to flush the cache before disabling the MMU and starting Linux proper >> >> 4. For historical reasons, 32-bit ARM Linux uses the S/W instructions to >> flush the cache. This still works on 32-bit hardware, and so the Linux >> maintainers are loathe to change it, even though more reliable VA-based >> instructions are available (?). > > It also works on 64bit HW. It is just not easily virtualizable, which is > why we've removed all S/W from the 64bit Linux port a while ago. From the diagram in your talk, it looked like the "flush the cache" operation *doesn't* work anywhere that has a "system cache", even on bare metal. >> 6. Rather than fix this in Linux, KVM has added a work-around in which >> the *hypervisor* flushes the caches at certain points (!!!). Julien is >> looking into doing the same with Xen. > > The "at certain points" doesn't quite describe it. We fully emulate S/W > instruction using the biggest hammer we can find. Oh, I thought Julien was saying something about flushing the guest's RAM every time caching was enabled or disabled. >> Given the variety of hardware that Linux has to run on, it's hard to >> understand why 1) 32-bit ARM Linux couldn't detect if it would be >> appropriate to use VA-based instructions rather than S/W instructions 2) >> There couldn't at least be a Kconfig option to use VA instructions >> instead of S/W instructions. > > [Linux hat on] > > 1) There is hardly anything to detect. Both sets of CMOs are available > on a moderately recent implementation. What you'd want to detect is the > the kernel is "virtualizable", which is not an easy task. > An alternative option would be to switch to VA CMOs if compiled for > ARMv7 (and maybe v6), assuming that doesn't have any horrible side > effect with broken cache implementations (and there is a few out there). > You'll have to check that this doesn't regress on any existing HW. So the idea would be to use the VA-based operations if available, and then special-case specific chipsets known to have issues. Linux (and Xen and...) end up doing this for lots of different kinds of hardware; this would be no different. > 2) Kconfig options are the way to hell. It took us 5 years to get a > 32
Re: [Xen-devel] [PATCH v3 19/25] x86emul: tell cmpxchg hook whether LOCK is in effect
> -Original Message- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: 07 December 2017 14:14 > To: xen-devel > Cc: Andrew Cooper ; Paul Durrant > ; George Dunlap ; > Tim (Xen.org) > Subject: [PATCH v3 19/25] x86emul: tell cmpxchg hook whether LOCK is in > effect > > This is necessary for the hook to correctly perform the operation. > > Signed-off-by: Jan Beulich Reviewed-by: Paul Durrant > --- > v3: New. > > --- a/tools/fuzz/x86_instruction_emulator/fuzz-emul.c > +++ b/tools/fuzz/x86_instruction_emulator/fuzz-emul.c > @@ -346,6 +346,7 @@ static int fuzz_cmpxchg( > void *old, > void *new, > unsigned int bytes, > +bool lock, > struct x86_emulate_ctxt *ctxt) > { > /* > --- a/tools/tests/x86_emulator/test_x86_emulator.c > +++ b/tools/tests/x86_emulator/test_x86_emulator.c > @@ -320,6 +320,7 @@ static int cmpxchg( > void *old, > void *new, > unsigned int bytes, > +bool lock, > struct x86_emulate_ctxt *ctxt) > { > if ( verbose ) > --- a/xen/arch/x86/hvm/emulate.c > +++ b/xen/arch/x86/hvm/emulate.c > @@ -1249,6 +1249,7 @@ static int hvmemul_cmpxchg_discard( > void *p_old, > void *p_new, > unsigned int bytes, > +bool lock, > struct x86_emulate_ctxt *ctxt) > { > return X86EMUL_OKAY; > @@ -1292,6 +1293,7 @@ static int hvmemul_cmpxchg( > void *p_old, > void *p_new, > unsigned int bytes, > +bool lock, > struct x86_emulate_ctxt *ctxt) > { > /* Fix this in case the guest is really relying on r-m-w atomicity. */ > --- a/xen/arch/x86/mm/shadow/common.c > +++ b/xen/arch/x86/mm/shadow/common.c > @@ -281,6 +281,7 @@ hvm_emulate_cmpxchg(enum x86_segment seg > void *p_old, > void *p_new, > unsigned int bytes, > +bool lock, > struct x86_emulate_ctxt *ctxt) > { > struct sh_emulate_ctxt *sh_ctxt = > --- a/xen/arch/x86/pv/ro-page-fault.c > +++ b/xen/arch/x86/pv/ro-page-fault.c > @@ -216,7 +216,7 @@ static int ptwr_emulated_write(enum x86_ > > static int ptwr_emulated_cmpxchg(enum x86_segment seg, unsigned long > offset, > void *p_old, void *p_new, unsigned int > bytes, > - struct x86_emulate_ctxt *ctxt) > + bool lock, struct x86_emulate_ctxt *ctxt) > { > paddr_t old = 0, new = 0; > > --- a/xen/arch/x86/x86_emulate/x86_emulate.c > +++ b/xen/arch/x86/x86_emulate/x86_emulate.c > @@ -1973,7 +1973,7 @@ protmode_load_seg( > > fail_if(!ops->cmpxchg); > switch ( (rc = ops->cmpxchg(sel_seg, (sel & 0xfff8) + 4, &desc.b, > -&new_desc_b, sizeof(desc.b), ctxt)) ) > +&new_desc_b, sizeof(desc.b), true, > ctxt)) ) > { > case X86EMUL_OKAY: > break; > @@ -6982,7 +6982,8 @@ x86_emulate( > } > > if ( (rc = ops->cmpxchg(ea.mem.seg, ea.mem.off, old, aux, > -op_bytes, ctxt)) != X86EMUL_OKAY ) > +op_bytes, lock_prefix, > +ctxt)) != X86EMUL_OKAY ) > goto done; > _regs.eflags |= X86_EFLAGS_ZF; > } > @@ -8434,7 +8435,7 @@ x86_emulate( > fail_if(!ops->cmpxchg); > rc = ops->cmpxchg( > dst.mem.seg, dst.mem.off, &dst.orig_val, > -&dst.val, dst.bytes, ctxt); > +&dst.val, dst.bytes, true, ctxt); > } > else > { > --- a/xen/arch/x86/x86_emulate/x86_emulate.h > +++ b/xen/arch/x86/x86_emulate/x86_emulate.h > @@ -237,10 +237,11 @@ struct x86_emulate_ops > struct x86_emulate_ctxt *ctxt); > > /* > - * cmpxchg: Emulate an atomic (LOCKed) CMPXCHG operation. > + * cmpxchg: Emulate a CMPXCHG operation. > * @p_old: [IN ] Pointer to value expected to be current at @addr. > * @p_new: [IN ] Pointer to value to write to @addr. > * @bytes: [IN ] Operation size (up to 8 (x86/32) or 16 (x86/64) bytes). > + * @lock: [IN ] atomic (LOCKed) operation > */ > int (*cmpxchg)( > enum x86_segment seg, > @@ -248,6 +249,7 @@ struct x86_emulate_ops > void *p_old, > void *p_new, > unsigned int bytes, > +bool lock, > struct x86_emulate_ctxt *ctxt); > > /* > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC Patch v4 2/8] ioreq: bump the number of IOREQ page to 4 pages
> -Original Message- > From: Chao Gao [mailto:chao@intel.com] > Sent: 07 December 2017 06:57 > To: Paul Durrant > Cc: Stefano Stabellini ; Wei Liu > ; Andrew Cooper ; Tim > (Xen.org) ; George Dunlap ; > xen-de...@lists.xen.org; Jan Beulich ; Ian Jackson > > Subject: Re: [RFC Patch v4 2/8] ioreq: bump the number of IOREQ page to 4 > pages > > On Thu, Dec 07, 2017 at 08:41:14AM +, Paul Durrant wrote: > >> -Original Message- > >> From: Xen-devel [mailto:xen-devel-boun...@lists.xenproject.org] On > Behalf > >> Of Paul Durrant > >> Sent: 06 December 2017 16:10 > >> To: 'Chao Gao' > >> Cc: Stefano Stabellini ; Wei Liu > >> ; Andrew Cooper ; > Tim > >> (Xen.org) ; George Dunlap ; > >> xen-de...@lists.xen.org; Jan Beulich ; Ian Jackson > >> > >> Subject: Re: [Xen-devel] [RFC Patch v4 2/8] ioreq: bump the number of > >> IOREQ page to 4 pages > >> > >> > -Original Message- > >> > From: Chao Gao [mailto:chao@intel.com] > >> > Sent: 06 December 2017 09:02 > >> > To: Paul Durrant > >> > Cc: xen-de...@lists.xen.org; Tim (Xen.org) ; Stefano > >> > Stabellini ; Konrad Rzeszutek Wilk > >> > ; Jan Beulich ; George > >> > Dunlap ; Andrew Cooper > >> > ; Wei Liu ; Ian > Jackson > >> > > >> > Subject: Re: [RFC Patch v4 2/8] ioreq: bump the number of IOREQ page > to 4 > >> > pages > >> > > >> > On Wed, Dec 06, 2017 at 03:04:11PM +, Paul Durrant wrote: > >> > >> -Original Message- > >> > >> From: Chao Gao [mailto:chao@intel.com] > >> > >> Sent: 06 December 2017 07:50 > >> > >> To: xen-de...@lists.xen.org > >> > >> Cc: Chao Gao ; Paul Durrant > >> > >> ; Tim (Xen.org) ; Stefano > >> > Stabellini > >> > >> ; Konrad Rzeszutek Wilk > >> > >> ; Jan Beulich ; > George > >> > >> Dunlap ; Andrew Cooper > >> > >> ; Wei Liu ; Ian > >> > Jackson > >> > >> > >> > >> Subject: [RFC Patch v4 2/8] ioreq: bump the number of IOREQ page > to 4 > >> > >> pages > >> > >> > >> > >> One 4K-byte page at most contains 128 'ioreq_t'. In order to remove > the > >> > vcpu > >> > >> number constraint imposed by one IOREQ page, bump the number > of > >> > IOREQ > >> > >> page to > >> > >> 4 pages. With this patch, multiple pages can be used as IOREQ page. > >> > >> > >> > >> Basically, this patch extends 'ioreq' field in struct hvm_ioreq_server > to > >> an > >> > >> array. All accesses to 'ioreq' field such as 's->ioreq' are replaced > >> > >> with > >> > >> FOR_EACH_IOREQ_PAGE macro. > >> > >> > >> > >> In order to access an IOREQ page, QEMU should get the gmfn and > map > >> > this > >> > >> gmfn > >> > >> to its virtual address space. > >> > > > >> > >No. There's no need to extend the 'legacy' mechanism of using magic > >> page > >> > gfns. You should only handle the case where the mfns are allocated on > >> > demand (see the call to hvm_ioreq_server_alloc_pages() in > >> > hvm_get_ioreq_server_frame()). The number of guest vcpus is known > at > >> > this point so the correct number of pages can be allocated. If the > >> > creator > of > >> > the ioreq server attempts to use the legacy > hvm_get_ioreq_server_info() > >> > and the guest has >128 vcpus then the call should fail. > >> > > >> > Great suggestion. I will introduce a new dmop, a variant of > >> > hvm_get_ioreq_server_frame() for creator to get an array of gfns and > the > >> > size of array. And the legacy interface will report an error if more > >> > than one IOREQ PAGES are needed. > >> > >> You don't need a new dmop for mapping I think. The mem op to map > ioreq > >> server frames should work. All you should need to do is update > >> hvm_get_ioreq_server_frame() to deal with an index > 1, and provide > some > >> means for the ioreq server creator to convert the number of guest vcpus > into > >> the correct number of pages to map. (That might need a new dm op). > > > >I realise after saying this that an emulator already knows the size of the > ioreq structure and so can easily calculate the correct number of pages to > map, given the number of guest vcpus. > > How about the patch in the bottom? Is it in the right direction? Yes, certainly along the right lines. I would probably do away with MAX_NR_IOREQ_PAGE though. You should just to dynamically allocate the correct number of ioreq pages when the ioreq server is created (since you already calculate nr_ioreq_page there anyway). > Do you have the QEMU patch, which replaces the old method with the new > method > to set up mapping? I want to integrate that patch and do some tests. Sure. There's a couple of patched. I have not tested them with recent rebases of my series so you may find some issues. Cheers, Paul 0001-Separate-ioreq-server-mapping-code-from-general-init.patch Description: 0001-Separate-ioreq-server-mapping-code-from-general-init.patch 0002-use-new-interface.patch Description: 0002-use-new-interface.patch ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-
Re: [Xen-devel] [PATCH v2 2/3] x86/acpi: take rsdp address for boot params if available
On 08/12/17 08:05, Ingo Molnar wrote: > > * Juergen Gross wrote: ... > acpi_physical_address acpi_arch_get_root_pointer(void) > { > return boot_params.hdr.acpi_rsdp_addr; > } > > 4) > > Add this to arch/x86/include/asm/acpi.h: > > extern acpi_physical_address acpi_arch_get_root_pointer(void); Uuh, this leads to problems for files including directly: acpi_physical_address won't be defined, and including from arch/x86/include/asm/acpi.h will lead to: #error unknown ACPI_MACHINE_WIDTH This can only be avoided by including from which seems to be the wrong layering. So I could: a) modify the sources including to use instead b) don't use acpi_physical_address but either u64 or unsigned long. c) ? What would be your preference? Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] x86/Xen: don't report ancient LAPIC version
Unconditionally reporting a value seen on the P4 or older invokes functionality like io_apic_get_unique_id() on 32-bit builds, resulting in a panic() with sufficiently many CPUs and/or IO-APICs. Doing what that function does would be the hypervisor's responsibility anyway, so makes no sense to be used when running on Xen. Uniformly report a more modern version; this shouldn't matter much as both LAPIC and IO-APIC are being managed entirely / mostly by the hypervisor. Signed-off-by: Jan Beulich --- a/arch/x86/xen/apic.c +++ b/arch/x86/xen/apic.c @@ -57,7 +57,7 @@ static u32 xen_apic_read(u32 reg) return 0; if (reg == APIC_LVR) - return 0x10; + return 0x14; #ifdef CONFIG_X86_32 if (reg == APIC_LDR) return SET_APIC_LOGICAL_ID(1UL << smp_processor_id()); ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 2/3] x86/acpi: take rsdp address for boot params if available
* Juergen Gross wrote: > On 08/12/17 08:05, Ingo Molnar wrote: > > > > * Juergen Gross wrote: > > ... > > > acpi_physical_address acpi_arch_get_root_pointer(void) > > { > > return boot_params.hdr.acpi_rsdp_addr; > > } > > > > 4) > > > > Add this to arch/x86/include/asm/acpi.h: > > > > extern acpi_physical_address acpi_arch_get_root_pointer(void); > > Uuh, this leads to problems for files including directly: > acpi_physical_address won't be defined, and including > from arch/x86/include/asm/acpi.h will lead to: > > #error unknown ACPI_MACHINE_WIDTH > > This can only be avoided by including from > which seems to be the wrong layering. > > So I could: > > a) modify the sources including to use >instead > b) don't use acpi_physical_address but either u64 or unsigned long. > c) ? > > What would be your preference? Would it help if you put the prototype into linux/acpi.h perhaps? It's a generic facility in principle, even if only used by x86 at the moment. Thanks, Ingo ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 2/3] x86/acpi: take rsdp address for boot params if available
On 08/12/17 12:26, Ingo Molnar wrote: > > * Juergen Gross wrote: > >> On 08/12/17 08:05, Ingo Molnar wrote: >>> >>> * Juergen Gross wrote: >> >> ... >> >>> acpi_physical_address acpi_arch_get_root_pointer(void) >>> { >>> return boot_params.hdr.acpi_rsdp_addr; >>> } >>> >>> 4) >>> >>> Add this to arch/x86/include/asm/acpi.h: >>> >>> extern acpi_physical_address acpi_arch_get_root_pointer(void); >> >> Uuh, this leads to problems for files including directly: >> acpi_physical_address won't be defined, and including >> from arch/x86/include/asm/acpi.h will lead to: >> >> #error unknown ACPI_MACHINE_WIDTH >> >> This can only be avoided by including from >> which seems to be the wrong layering. >> >> So I could: >> >> a) modify the sources including to use >>instead >> b) don't use acpi_physical_address but either u64 or unsigned long. >> c) ? >> >> What would be your preference? > > Would it help if you put the prototype into linux/acpi.h perhaps? It's a > generic > facility in principle, even if only used by x86 at the moment. Yes, that seems to work. Thanks, Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC PATCH 06/31] cpufreq: make cpufreq driver more generalizable
Hi Jan On Fri, Dec 8, 2017 at 10:07 AM, Jan Beulich wrote: On 07.12.17 at 21:31, wrote: >> Have questions which need to be clarified: >> >> If I understood correctly, new variant of set_px_pminfo is going to >> have an extra "flag" argument, since >> struct processor_performance doesn't have "flag" field (it contains >> "state" field instead, which has yet another meaning). >> Something like that: >> int set_px_pminfo(uint32_t acpi_id, uint32_t flag, struct >> processor_performance *dom0_px_info) >> Is my understanding correct? > > Well, you obviously must not lose information, so having that > extra parameter is unavoidable. Please use common sense > when dealing with such re-structuring. And btw, please also be > precise: There's no "flag" field, but there is a "flags" one. Such > should also be the name of the new parameter - we're talking > about multiple bits here, after all. Indeed "flags", sorry for being unclear. > >> As for set_cx_pminfo(). To what struct we should do translation from >> struct xen_processor_power? (struct acpi_processor_power?) > > Yes, of course. > >> Briefly looking at set_cx_pminfo(), I got a feeling, that in order to >> modify it in a "set_px_pminfo() manner" >> we need to rework print_cx_pminfo(), set_cx(), check_cx(), >> acpi_processor_ffh_cstate_probe() too, since >> all these function have arguments which contain XEN_GUEST_HANDLE. I am >> wondering is it worth >> doing such rework taking into the account that set_cx_pminfo() is not >> going to be called from the non-hypercall context. >> Or I missed something? > > Without looking at the details of this, please again use common > sense. If there are good reasons for the two functions to not > follow the same model, please simply state so in the overview > mail of the patch series and/or (briefly, but concisely) in the > specific patch's description. A good reason for example would > be if overly large amounts of other code would need touching. Agree. > > Jan > -- Regards, Oleksandr Tyshchenko ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v8] x86/altp2m: support for setting restrictions for an array of pages
>>> On 24.10.17 at 12:19, wrote: > HVMOP_altp2m_set_mem_access_multi has been added as a HVMOP (as opposed to a > DOMCTL) for consistency with its HVMOP_altp2m_set_mem_access counterpart (and > hence with the original altp2m design, where domains are allowed - with the > proper altp2m access rights - to alter these settings), in the absence of an > official position on the issue from the original altp2m designers. I continue to disagree with this reasoning. I'm afraid I'm not really willing to allow widening the badness, unless altp2m was formally documented security-unsupported. > +int xc_altp2m_set_mem_access_multi(xc_interface *xch, uint32_t domid, > + uint16_t view_id, uint8_t *access, > + uint64_t *pages, uint32_t nr) > +{ > +int rc; > + > +DECLARE_HYPERCALL_BUFFER(xen_hvm_altp2m_op_t, arg); > +DECLARE_HYPERCALL_BOUNCE(access, nr, XC_HYPERCALL_BUFFER_BOUNCE_IN); > +DECLARE_HYPERCALL_BOUNCE(pages, nr * sizeof(uint64_t), > + XC_HYPERCALL_BUFFER_BOUNCE_IN); This is confusing: Why "* sizeof()" in the latter expression, but not in the former. It would anyway be better to use sizeof(*pages) (and then also sizeof(*access)) to clearly make the connection. > @@ -4568,6 +4571,37 @@ static int do_altp2m_op( > a.u.set_mem_access.view); > break; > > +case HVMOP_altp2m_set_mem_access_multi: > +if ( a.u.set_mem_access_multi.pad || > + a.u.set_mem_access_multi.opaque >= a.u.set_mem_access_multi.nr ) > +{ > +rc = -EINVAL; > +break; > +} Just like in a number of other recent cases: This operation should not fail when .nr is zero. Yet as per above it will. > +static int compat_altp2m_op( > +XEN_GUEST_HANDLE_PARAM(void) arg) > +{ > +int rc = 0; > +struct compat_hvm_altp2m_op a; > +union > +{ > +XEN_GUEST_HANDLE_PARAM(void) hnd; > +struct xen_hvm_altp2m_op *altp2m_op; > +} nat; > + > +if ( !hvm_altp2m_supported() ) > +return -EOPNOTSUPP; > + > +if ( copy_from_guest(&a, arg, 1) ) > +return -EFAULT; > + > +if ( a.pad1 || a.pad2 || > + (a.version != HVMOP_ALTP2M_INTERFACE_VERSION) ) > +return -EINVAL; > + > +set_xen_guest_handle(nat.hnd, COMPAT_ARG_XLAT_VIRT_BASE); > + > +switch ( a.cmd ) > +{ > +case HVMOP_altp2m_set_mem_access_multi: > +#define XLAT_hvm_altp2m_set_mem_access_multi_HNDL_pfn_list(_d_, _s_); \ > +guest_from_compat_handle((_d_)->pfn_list, (_s_)->pfn_list) > +#define XLAT_hvm_altp2m_set_mem_access_multi_HNDL_access_list(_d_, _s_); \ > +guest_from_compat_handle((_d_)->access_list, (_s_)->access_list) > + > XLAT_hvm_altp2m_set_mem_access_multi(&nat.altp2m_op->u.set_mem_access_multi, > + &a.u.set_mem_access_multi); > +#undef XLAT_hvm_altp2m_set_mem_access_multi_HNDL_pfn_list > +#undef XLAT_hvm_altp2m_set_mem_access_multi_HNDL_access_list > +break; > +default: Blank line between individual case blocks please (also below). > --- a/xen/include/Makefile > +++ b/xen/include/Makefile > @@ -28,6 +28,7 @@ headers-$(CONFIG_X86) += compat/arch-x86/xen.h > headers-$(CONFIG_X86) += compat/arch-x86/xen-$(compat-arch-y).h > headers-$(CONFIG_X86) += compat/hvm/hvm_vcpu.h > headers-$(CONFIG_X86) += compat/hvm/dm_op.h > +headers-$(CONFIG_X86) += compat/hvm/hvm_op.h I realize the dm_op.h insertion was already to the wrong spot, but please let's not make things worse: Anywhere that order doesn't otherwise matter, please sort alphabetically, to prevent everyone adding to the end (and hence increasing the risk of patch conflicts). > @@ -237,6 +246,20 @@ struct xen_hvm_altp2m_set_mem_access { > typedef struct xen_hvm_altp2m_set_mem_access xen_hvm_altp2m_set_mem_access_t; > DEFINE_XEN_GUEST_HANDLE(xen_hvm_altp2m_set_mem_access_t); > > +struct xen_hvm_altp2m_set_mem_access_multi { > +/* view */ > +uint16_t view; > +uint16_t pad; > +/* Number of pages */ > +uint32_t nr; > +/* Used for continuation purposes */ > +uint64_t opaque; The comment should also state that this needs to be set to zero upon initial invocation. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 07/13] iommu: Make decision about needing IOMMU for hardware domains in advance
Hi, Jan On Thu, Dec 7, 2017 at 3:57 PM, Jan Beulich wrote: On 07.12.17 at 14:50, wrote: >> On Thu, Dec 7, 2017 at 10:57 AM, Jan Beulich wrote: >> On 06.12.17 at 20:23, wrote: On Wed, Dec 6, 2017 at 7:01 PM, Jan Beulich wrote: On 25.07.17 at 19:26, wrote: >> @@ -175,37 +182,6 @@ void __hwdom_init iommu_hwdom_init(struct domain *d) >> return; >> >> register_keyhandler('o', &iommu_dump_p2m_table, "dump iommu p2m >> table", 0); >> -d->need_iommu = !!iommu_dom0_strict; >> -if ( need_iommu(d) && !iommu_use_hap_pt(d) ) >> -{ >> -struct page_info *page; >> -unsigned int i = 0; >> -int rc = 0; >> - >> -page_list_for_each ( page, &d->page_list ) >> -{ >> -unsigned long mfn = page_to_mfn(page); >> -unsigned long gfn = mfn_to_gmfn(d, mfn); >> -unsigned int mapping = IOMMUF_readable; >> -int ret; >> - >> -if ( ((page->u.inuse.type_info & PGT_count_mask) == 0) || >> - ((page->u.inuse.type_info & PGT_type_mask) >> - == PGT_writable_page) ) >> -mapping |= IOMMUF_writable; >> - >> -ret = hd->platform_ops->map_pages(d, gfn, mfn, 0, mapping); >> -if ( !rc ) >> -rc = ret; >> - >> -if ( !(i++ & 0xf) ) >> -process_pending_softirqs(); >> -} >> - >> -if ( rc ) >> -printk(XENLOG_WARNING "d%d: IOMMU mapping failed: %d\n", >> - d->domain_id, rc); >> -} >> >> return hd->platform_ops->hwdom_init(d); >> } > > Just to double check - this change was tested on x86 Dom0, at > least PV (for PVH I'd at least expect that you've did some static > code analysis to make sure this doesn't put in further roadblocks)? I am afraid I didn't get the second part of this sentence. >>> >>> Understandably, since I've broken grammar in the course of >>> re-phrasing a number of times before sending. Dom0 PVH isn't >>> complete at this point, so I can't ask you to actually test it. But >>> I want to be reasonably certain that the change you make won't >>> further complicate this enabling work (you may want to also Cc >>> Roger on future versions of the patch for this very reason), the >>> more that on AMD we've been unconditionally using non-shared >>> page tables for quite some time. (In fact I see chances that the >>> change might actually help the time it takes to set up PVH Dom0, >>> especially when a sufficiently large chunk of memory is being >>> handed to it.) >> >> As I understand, the current patch was tested on x86 with PV dom0 >> (thanks for doing that), > > This sounds as if you believe I would have tested anything. I > certainly didn't (or at least I don't recall), and never meant to. > >> but wasn't >> tested with PVH dom0 since the latter wasn't ready. And there is some >> activity for bringing PVH dom0 >> which the current patch may affect in a negative way (complicate, brake, >> etc). >> >> What pending patch(es) or a part of already existing code on x86 >> should I pay special attention to? > > The question is not so much pending patches, but making sure your > changes don't adversely affect what's already in the tree. Sure. > Beyond that I'll defer to Roger. > >> Sorry for the maybe naive question, but what should be done from my >> side for this patch to be accepted, >> except addressing comments regarding the patch itself? > > You will want (need) to assess the impact of your changes on > code paths you can't possibly test. Sure. I would like to clarify: I haven't tested this patch on x86. Only on ARM. > > Jan > -- Regards, Oleksandr Tyshchenko ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v8] x86/altp2m: support for setting restrictions for an array of pages
On 12/08/2017 02:18 PM, Jan Beulich wrote: On 24.10.17 at 12:19, wrote: >> HVMOP_altp2m_set_mem_access_multi has been added as a HVMOP (as opposed to a >> DOMCTL) for consistency with its HVMOP_altp2m_set_mem_access counterpart (and >> hence with the original altp2m design, where domains are allowed - with the >> proper altp2m access rights - to alter these settings), in the absence of an >> official position on the issue from the original altp2m designers. > > I continue to disagree with this reasoning. I'm afraid I'm not really > willing to allow widening the badness, unless altp2m was formally > documented security-unsupported. Going the DOMCTL route here would have been the (much easier) solution, and in fact, as stated before, there has been an attempt to do so - however, IIRC Andrew has insisted that we should take care to use consistent access privilege across altp2m operations. This was followed by a lengthy xen-devel discussion and several unsuccessful attempts to obtain an official position from the original contributors, at which point (after several months), as also discussed at the Xen Developer Summit in Budapest, we decided to press on in the direction that had seemed the most compatible with the original altp2m design. (Please correct me if I'm misremembering or misunderstanding something.) So at this point it looks like we're stuck again: we're happy to go in any direction the maintainers decide is the best, but we do need to decide on one. FWIW, Tamas (CC added) has added code to restrict where altp2m calls can come from (although that's not XSM code). Please let us know how to proceed. >> +int xc_altp2m_set_mem_access_multi(xc_interface *xch, uint32_t domid, >> + uint16_t view_id, uint8_t *access, >> + uint64_t *pages, uint32_t nr) >> +{ >> +int rc; >> + >> +DECLARE_HYPERCALL_BUFFER(xen_hvm_altp2m_op_t, arg); >> +DECLARE_HYPERCALL_BOUNCE(access, nr, XC_HYPERCALL_BUFFER_BOUNCE_IN); >> +DECLARE_HYPERCALL_BOUNCE(pages, nr * sizeof(uint64_t), >> + XC_HYPERCALL_BUFFER_BOUNCE_IN); > > This is confusing: Why "* sizeof()" in the latter expression, but not > in the former. It would anyway be better to use sizeof(*pages) (and > then also sizeof(*access)) to clearly make the connection. I've opted for less clutter here, since of course sizeof(uint8_t) == 1, so nr == nr * sizeof(uint8_t). But we're happy to make the change, it will indeed make the code clearer. >> @@ -4568,6 +4571,37 @@ static int do_altp2m_op( >> a.u.set_mem_access.view); >> break; >> >> +case HVMOP_altp2m_set_mem_access_multi: >> +if ( a.u.set_mem_access_multi.pad || >> + a.u.set_mem_access_multi.opaque >= a.u.set_mem_access_multi.nr >> ) >> +{ >> +rc = -EINVAL; >> +break; >> +} > > Just like in a number of other recent cases: This operation should not > fail when .nr is zero. Yet as per above it will. We'll test that .nr != 0 before the >= comparison. Thanks, Razvan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/Xen: don't report ancient LAPIC version
On 08/12/17 12:17, Jan Beulich wrote: > Unconditionally reporting a value seen on the P4 or older invokes > functionality like io_apic_get_unique_id() on 32-bit builds, resulting > in a panic() with sufficiently many CPUs and/or IO-APICs. Doing what > that function does would be the hypervisor's responsibility anyway, so > makes no sense to be used when running on Xen. Uniformly report a more > modern version; this shouldn't matter much as both LAPIC and IO-APIC are > being managed entirely / mostly by the hypervisor. > > Signed-off-by: Jan Beulich Reviewed-by: Juergen Gross BTW: Cc: stable? Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC] xen/arm: Handling cache maintenance instructions by set/way
On 08/12/17 08:03, Tim Deegan wrote: Hi, Hi Tim, Somehow your e-mail was marked as spam by gmail. At 12:58 + on 06 Dec (1512565090), Julien Grall wrote: On 12/06/2017 12:28 PM, George Dunlap wrote: 2. It sounds like rather than using PoD, you could use the "misconfigured p2m table" technique that x86 uses: set bits in the p2m entry which cause a specific kind of HAP fault when accessed. The fault handler then looks in the p2m entry, and if it finds an otherwise valid entry, it just fixes the "misconfigured" bits and continues. I thought about this. But when do you set the entry to misconfigured? If you take the example of Linux 32-bit. There are a couple of full cache clean during the boot of uni-processor. So you would need to go through the p2m multiple time and reset the access bits. My 2c (echoing what some others have already said): +1 for avoiding the full majesty of PoD if you don't need it. It should be possible to do something like the misconfigured-entry bit trick by _allocating_ the memory up-front and building the p2m entries but only making them usable by the {IO}MMUs on first access. That would make these early p2m walks shorter (because they can skip whole subtrees that aren't marked present yet) without making major changes to domain build or introducing run-time failures. I am not aware of any way on Arm to misconfigure an entry. We do have valid and access bits, although they will affect the IOMMU as well. So it will not be possible to get page-table sharing with this "feature" enabled. At the moment, I am thinking to provide a per-guest option to turn on/off the possibility to use the valid/access bit. That will be at the expense to do a full invalidate on S/W. Also beware of DoS conditions -- a guest that touches all its memory and then flushes by set/way mustn't be allowed to hurt the rest of the system. That probably means the set/way flush has to be preemptable. I am fully aware about it :). This was actually mentioned in my first e-mail. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] linux-xen-arm branch update
On 05/12/17 18:42, Julien Grall wrote: Hi Ian, On 05/12/17 18:41, Ian Jackson wrote: Stefano Stabellini writes ("Re: [OSSTEST PATCH] linux-arm-xen: Get from shared arm/linux.git xenbits tree"): On Tue, 5 Dec 2017, Julien Grall wrote: Acked-by: Julien Grall Acked-by: Stefano Stabellini Thanks. This queued for my next osstest push, which I expect to do some time tomorrow. Thank you! Once we get the push, I will update the tree to the latest 4.9 (we are using an ancient version). FIY, we got a push. So I have updated to the latest 4.9.y. Cheers, Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 3/4] x86/xen: fix boot loader version reported for pvh guests
The boot loader version reported via sysfs is wrong in case of the kernel being booted via the Xen PVH boot entry. it should be 2.12 (0x020c), but it is reported to be 2.18 (0x0212). As the current way to set the version is error prone use the more readable variant (2 << 8) | 12. Cc: # 4.12 Signed-off-by: Juergen Gross --- arch/x86/xen/enlighten_pvh.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/xen/enlighten_pvh.c b/arch/x86/xen/enlighten_pvh.c index 436c4f003e17..6e6430cb5e3f 100644 --- a/arch/x86/xen/enlighten_pvh.c +++ b/arch/x86/xen/enlighten_pvh.c @@ -69,7 +69,7 @@ static void __init init_pvh_bootparams(void) * Version 2.12 supports Xen entry point but we will use default x86/PC * environment (i.e. hardware_subarch 0). */ - pvh_bootparams.hdr.version = 0x212; + pvh_bootparams.hdr.version = (2 << 8) | 12; pvh_bootparams.hdr.type_of_loader = (9 << 4) | 0; /* Xen loader */ } -- 2.12.3 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 1/4] x86/boot: add acpi rsdp address to setup_header
Xen PVH guests receive the address of the RSDP table from Xen. In order to support booting a Xen PVH guest via Grub2 using the standard x86 boot entry we need a way for Grub2 to pass the RSDP address to the kernel. For this purpose expand the struct setup_header to hold the physical address of the RSDP address. Being zero means it isn't specified and has to be located the legacy way (searching through low memory or EBDA). While documenting the new setup_header layout and protocol version 2.14 add the missing documentation of protocol version 2.13. Signed-off-by: Juergen Gross Reviewed-by: Roger Pau Monné --- V3: fix commit message and some documentation bits (Ingo Molnar) --- Documentation/x86/boot.txt| 19 +++ arch/x86/boot/header.S| 6 +- arch/x86/include/uapi/asm/bootparam.h | 1 + 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/Documentation/x86/boot.txt b/Documentation/x86/boot.txt index 5e9b826b5f62..cec112909c35 100644 --- a/Documentation/x86/boot.txt +++ b/Documentation/x86/boot.txt @@ -61,6 +61,13 @@ Protocol 2.12: (Kernel 3.8) Added the xloadflags field and extension fields to struct boot_params for loading bzImage and ramdisk above 4G in 64bit. +Protocol 2.13: (Kernel 3.14) Support 32- and 64-bit flags being set in + xloadflags to support booting a 64-bit kernel from 32-bit + EFI + +Protocol 2.14 (Kernel 4.16) Added acpi_rsdp_addr holding the physical + address of the ACPI RSDP table. + MEMORY LAYOUT The traditional memory map for the kernel loader, used for Image or @@ -197,6 +204,7 @@ Offset Proto NameMeaning 0258/8 2.10+ pref_addressPreferred loading address 0260/4 2.10+ init_size Linear memory required during initialization 0264/4 2.11+ handover_offset Offset of handover entry point +0268/8 2.14+ acpi_rsdp_addr Physical address of RSDP table (1) For backwards compatibility, if the setup_sects field contains 0, the real value is 4. @@ -744,6 +752,17 @@ Offset/size: 0x264/4 See EFI HANDOVER PROTOCOL below for more details. +Field name:acpi_rsdp_addr +Type: write +Offset/size: 0x268/8 +Protocol: 2.14+ + + This field can be set by the boot loader to tell the kernel the + physical address of the ACPI RSDP table. + + A value of 0 indicates the kernel should fall back to the standard + methods to locate the RSDP. + THE IMAGE CHECKSUM diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S index 850b8762e889..4c881c850125 100644 --- a/arch/x86/boot/header.S +++ b/arch/x86/boot/header.S @@ -300,7 +300,7 @@ _start: # Part 2 of the header, from the old setup.S .ascii "HdrS" # header signature - .word 0x020d # header version number (>= 0x0105) + .word 0x020e # header version number (>= 0x0105) # or else old loadlin-1.5 will fail) .globl realmode_swtch realmode_swtch:.word 0, 0# default_switch, SETUPSEG @@ -558,6 +558,10 @@ pref_address: .quad LOAD_PHYSICAL_ADDR # preferred load addr init_size: .long INIT_SIZE # kernel initialization size handover_offset: .long 0 # Filled in by build.c +acpi_rsdp_addr:.quad 0 # 64-bit physical pointer to the + # ACPI RSDP table, added with + # version 2.14 + # End of setup header # .section ".entrytext", "ax" diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h index afdd5ae0fcc4..5742e433e93e 100644 --- a/arch/x86/include/uapi/asm/bootparam.h +++ b/arch/x86/include/uapi/asm/bootparam.h @@ -85,6 +85,7 @@ struct setup_header { __u64 pref_address; __u32 init_size; __u32 handover_offset; + __u64 acpi_rsdp_addr; } __attribute__((packed)); struct sys_desc_table { -- 2.12.3 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 4/4] x86/xen: supply rsdp address in boot params for pvh guests
When booted via the special PVH entry save the RSDP address set in the boot information block in struct boot_params. This will enable Xen to locate the RSDP at an arbitrary address. Set the boot loader version to 2.14 (0x020e) replacing the wrong 0x0212 which should have been 0x020c. Signed-off-by: Juergen Gross --- V2: set bootloader version to 2.14 (Roger Pau Monné) --- arch/x86/xen/enlighten_pvh.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/arch/x86/xen/enlighten_pvh.c b/arch/x86/xen/enlighten_pvh.c index 6e6430cb5e3f..e85e6dafe4bc 100644 --- a/arch/x86/xen/enlighten_pvh.c +++ b/arch/x86/xen/enlighten_pvh.c @@ -68,9 +68,12 @@ static void __init init_pvh_bootparams(void) * * Version 2.12 supports Xen entry point but we will use default x86/PC * environment (i.e. hardware_subarch 0). +* The RSDP address is available from version 2.14 on. */ - pvh_bootparams.hdr.version = (2 << 8) | 12; + pvh_bootparams.hdr.version = (2 << 8) | 14; pvh_bootparams.hdr.type_of_loader = (9 << 4) | 0; /* Xen loader */ + + pvh_bootparams.hdr.acpi_rsdp_addr = pvh_start_info.rsdp_paddr; } /* -- 2.12.3 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 0/4] x86: make rsdp address accessible via boot params
In the non-EFI boot path the ACPI RSDP table is currently found via either EBDA or by searching through low memory for the RSDP magic. This requires the RSDP to be located in the first 1MB of physical memory. Xen PVH guests, however, get the RSDP address via the start of day information block. In order to support an arbitrary RSDP address this patch series adds the physical address of the RSDP to the boot params structure filled by the boot loader. A kernel booted directly in PVH mode can save the RSDP address in the boot params, while a kernel booted in PVH mode via grub can rely on the RSDP address being specified by grub2 (which in turn got the address via the start of day information block from Xen). Juergen Gross (4): x86/boot: add acpi rsdp address to setup_header x86/acpi: take rsdp address for boot params if available x86/xen: fix boot loader version reported for pvh guests x86/xen: supply rsdp address in boot params for pvh guests Documentation/x86/boot.txt| 19 +++ arch/x86/boot/header.S| 6 +- arch/x86/include/uapi/asm/bootparam.h | 1 + arch/x86/kernel/acpi/boot.c | 7 +++ arch/x86/xen/enlighten_pvh.c | 5 - drivers/acpi/osl.c| 10 +- include/linux/acpi.h | 2 ++ 7 files changed, 47 insertions(+), 3 deletions(-) -- 2.12.3 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 2/4] x86/acpi: take rsdp address for boot params if available
In case the rsdp address in struct boot_params is specified don't try to find the table by searching, but take the address directly as set by the boot loader. Signed-off-by: Juergen Gross --- V3: use a generic retrieval function with a __weak annotated default function (Ingo Molnar) --- arch/x86/kernel/acpi/boot.c | 7 +++ drivers/acpi/osl.c | 10 +- include/linux/acpi.h| 2 ++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c index f4c463df8b08..26fc8972dc4b 100644 --- a/arch/x86/kernel/acpi/boot.c +++ b/arch/x86/kernel/acpi/boot.c @@ -47,6 +47,7 @@ #include #include #include +#include #include "sleep.h" /* To include x86_acpi_suspend_lowlevel */ static int __initdata acpi_force = 0; @@ -1758,3 +1759,9 @@ void __init arch_reserve_mem_area(acpi_physical_address addr, size_t size) e820__range_add(addr, size, E820_TYPE_ACPI); e820__update_table_print(); } + +acpi_physical_address acpi_arch_get_root_pointer(void) +{ + return boot_params.hdr.acpi_rsdp_addr; +} +EXPORT_SYMBOL_GPL(acpi_arch_get_root_pointer); diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c index 3bb46cb24a99..2b77db914752 100644 --- a/drivers/acpi/osl.c +++ b/drivers/acpi/osl.c @@ -178,6 +178,11 @@ void acpi_os_vprintf(const char *fmt, va_list args) #endif } +__weak acpi_physical_address acpi_arch_get_root_pointer(void) +{ + return 0; +} + #ifdef CONFIG_KEXEC static unsigned long acpi_rsdp; static int __init setup_acpi_rsdp(char *arg) @@ -189,12 +194,15 @@ early_param("acpi_rsdp", setup_acpi_rsdp); acpi_physical_address __init acpi_os_get_root_pointer(void) { - acpi_physical_address pa = 0; + acpi_physical_address pa; #ifdef CONFIG_KEXEC if (acpi_rsdp) return acpi_rsdp; #endif + pa = acpi_arch_get_root_pointer(); + if (pa) + return pa; if (efi_enabled(EFI_CONFIG_TABLES)) { if (efi.acpi20 != EFI_INVALID_TABLE_ADDR) diff --git a/include/linux/acpi.h b/include/linux/acpi.h index dc1ebfeeb5ec..aa603cc5ad30 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -1266,4 +1266,6 @@ static inline int lpit_read_residency_count_address(u64 *address) } #endif +acpi_physical_address acpi_arch_get_root_pointer(void); + #endif /*_LINUX_ACPI_H*/ -- 2.12.3 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/arm: bootfdt: Use proper default for #address-cells and #size-cells
Hi, On 29/11/17 18:12, Stefano Stabellini wrote: On Wed, 29 Nov 2017, Julien Grall wrote: Per the device-tree specific [1], when the property #address-cells and #size-cells are not present, the default value should be resp. 1 and 2. [1] https://www.devicetree.org/downloads/devicetree-specification-v0.1-20160524.pdf Signed-off-by: Julien Grall Acked-by: Stefano Stabellini This was acked but not applied to staging. Can you do it please? Cheers, --- This was discovered debugging DT generated by GRUB on ACPI-only platform. I am not aware of any DT relying on that for now, but it would still be nice to be compliant with the spec and avoid surprise. --- xen/arch/arm/bootfdt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c index 4a687e725d..8eba42c7b9 100644 --- a/xen/arch/arm/bootfdt.c +++ b/xen/arch/arm/bootfdt.c @@ -109,8 +109,8 @@ int __init device_tree_for_each_node(const void *fdt, continue; } -as = depth > 0 ? address_cells[depth-1] : 0; -ss = depth > 0 ? size_cells[depth-1] : 0; +as = depth > 0 ? address_cells[depth-1] : DT_ROOT_NODE_ADDR_CELLS_DEFAULT; +ss = depth > 0 ? size_cells[depth-1] : DT_ROOT_NODE_SIZE_CELLS_DEFAULT; address_cells[depth] = device_tree_get_u32(fdt, node, "#address-cells", as); -- 2.11.0 -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/arm: Surround HSR_SYSREG macro value with ()
Hi, Ping? Cheers, On 29/11/17 17:46, Julien Grall wrote: The value of the macro HCR_SYSREG is not surrounded by (). This means the behavior may change depend on how it is used. Thanksfully recent GCC will issue a warning for that. Signed-off-by: Julien Grall --- I am not aware of any "bad" usage today in Xen. This was found whilst playing with sysreg emulation and GCC complaining about the missing (). --- xen/include/asm-arm/arm64/sysregs.h | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/xen/include/asm-arm/arm64/sysregs.h b/xen/include/asm-arm/arm64/sysregs.h index 084d2a1e5d..1811234249 100644 --- a/xen/include/asm-arm/arm64/sysregs.h +++ b/xen/include/asm-arm/arm64/sysregs.h @@ -32,11 +32,11 @@ /* These are used to decode traps with HSR.EC==HSR_EC_SYSREG */ #define HSR_SYSREG(op0,op1,crn,crm,op2) \ -((__HSR_SYSREG_##op0) << HSR_SYSREG_OP0_SHIFT) | \ -((__HSR_SYSREG_##op1) << HSR_SYSREG_OP1_SHIFT) | \ -((__HSR_SYSREG_##crn) << HSR_SYSREG_CRN_SHIFT) | \ -((__HSR_SYSREG_##crm) << HSR_SYSREG_CRM_SHIFT) | \ -((__HSR_SYSREG_##op2) << HSR_SYSREG_OP2_SHIFT) +(((__HSR_SYSREG_##op0) << HSR_SYSREG_OP0_SHIFT) | \ + ((__HSR_SYSREG_##op1) << HSR_SYSREG_OP1_SHIFT) | \ + ((__HSR_SYSREG_##crn) << HSR_SYSREG_CRN_SHIFT) | \ + ((__HSR_SYSREG_##crm) << HSR_SYSREG_CRM_SHIFT) | \ + ((__HSR_SYSREG_##op2) << HSR_SYSREG_OP2_SHIFT)) #define HSR_SYSREG_DCISW HSR_SYSREG(1,0,c7,c6,2) #define HSR_SYSREG_DCCSW HSR_SYSREG(1,0,c7,c10,2) -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/arm: gic-v3: Bail out if gicv3_cpu_init fail
Hi Stefano, On 07/12/17 23:05, Stefano Stabellini wrote: On Wed, 6 Dec 2017, Julien Grall wrote: From: Julien Grall When system registers are not enabled, all the access to them will trap ^ accesses in EL2. In Xen, system registers will be enabled by gicv3_cpu_init only on success. As the rest of the code (e.g gicv3_hyp_init) relies on system register, it is better to bail out directly. This will save time on debugging early boot issue on GICv3 platform. Signed-off-by: Julien Grall This is good: Reviewed-by: Stefano Stabellini Do we also want to print a warning or an error message? AFAICT, all the path that return an error in gicv3_cpu_init (and the callers) have already a warning/error message. So no need to add an extra one here. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-next 06/16] xen/arm: Extend copy_to_guest to support copying from/to guest physical address
Hi Stefano, On 07/12/17 23:01, Stefano Stabellini wrote: On Wed, 6 Dec 2017, Julien Grall wrote: Hi Stefano, On 12/06/2017 01:22 AM, Stefano Stabellini wrote: On Thu, 23 Nov 2017, Julien Grall wrote: The only differences between copy_to_guest and access_guest_memory_by_ipa are: - The latter does not support copying data crossing page boundary - The former is copying from/to guest VA whilst the latter from guest PA copy_to_guest can easily be extended to support copying from/to guest physical address. For that a new bit is used to tell whether linear address or ipa is been used. Lastly access_guest_memory_by_ipa is reimplemented using copy_to_guest. This also has the benefits to extend the use of it, it is now possible to copy data crossing page boundary. Signed-off-by: Julien Grall Ah! This is the reason why previous patches were not using vaddr_t. It makes sense now. May I suggest we use something different from paddr_t in copy_guest for addr type? I don't think is correct to specify addr as paddr_t when it could be vaddr_t; in the future we could have type checks on them. I suggest we specify it as u64, but if you have a better idea go for it. We should not use more u64 in the code. uint64_t could be a solution but even that, I don't see the reason. How are you sure the physical address will always fit in 64-bit? On the other side, very likely vaddr_t will fit in paddr_t. So paddr_t is the right way to go for me. What about introducing xaddr_t? I would prefer uint64_t in that case. xaddr_t is quite confusing to read and could be misused. Or at least: static struct page_info *translate_get_page(struct vcpu *v, paddr_t /*or vaddr_t */ addr I can do that as well. What's your preference? Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-next 07/16] xen/arm: Introduce copy_to_guest_phys_flush_dcache
Hi, On 06/12/17 12:27, Julien Grall wrote: On 12/06/2017 01:26 AM, Stefano Stabellini wrote: On Thu, 23 Nov 2017, Julien Grall wrote: Hi Andrew, On 23/11/17 18:49, Andrew Cooper wrote: On 23/11/17 18:32, Julien Grall wrote: This new function will be used in a follow-up patch to copy data to the guest using the IPA (aka guest physical address) and then clean the cache. Signed-off-by: Julien Grall --- xen/arch/arm/guestcopy.c | 10 ++ xen/include/asm-arm/guest_access.h | 6 ++ 2 files changed, 16 insertions(+) diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c index be53bee559..7958663970 100644 --- a/xen/arch/arm/guestcopy.c +++ b/xen/arch/arm/guestcopy.c @@ -110,6 +110,16 @@ unsigned long raw_copy_from_guest(void *to, const void __user *from, unsigned le COPY_from_guest | COPY_linear); } +unsigned long copy_to_guest_phys_flush_dcache(struct domain *d, + paddr_t gpa, + void *buf, + unsigned int len) +{ + /* P2M is shared between all vCPUs, so the vCPU used does not matter. */ Be very careful with this line of thinking. It is only works after DOMCTL_max_vcpus has succeeded, and before that point, it is a latent NULL pointer dereference. I really don't expect that function been used before DOMCT_max_vcpus is set. It is only used for hardware emulation or Xen loading image into the hardware domain memory. I could add a check d->vcpus to be safe. Also, what about vcpus configured with alternative views? It is not important because the underlying call is get_page_from_gfn that does not care about the alternative view (that function take a domain in parameter). I can update the comment. Since this is a new function, would it make sense to take a struct vcpu* as parameter, instead of a struct domain* ? Well, I suggested this patch this way because likely everyone will use with d->vcpus[0]. And then you would have to wonder why d->vcpus[0] and not d->vcpus[1]... Thinking a bit more to this, it might be better/safer to pass either a domain or a vCPU to copy_guest. I can see 2 solutions: 1# Introduce a union that use the same parameter: union { struct { struct domain *d; } ipa; struct { struct vcpu *v; } gva; } The structure here would be to ensure that it is clear that only domain (resp. vcpu) should be used with ipa (resp. gva). 2# Have 2 parameters, vcpu and domain. Any opinions? Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v8] x86/altp2m: support for setting restrictions for an array of pages
On Fri, Dec 8, 2017 at 5:42 AM, Razvan Cojocaru wrote: > On 12/08/2017 02:18 PM, Jan Beulich wrote: > On 24.10.17 at 12:19, wrote: >>> HVMOP_altp2m_set_mem_access_multi has been added as a HVMOP (as opposed to a >>> DOMCTL) for consistency with its HVMOP_altp2m_set_mem_access counterpart >>> (and >>> hence with the original altp2m design, where domains are allowed - with the >>> proper altp2m access rights - to alter these settings), in the absence of an >>> official position on the issue from the original altp2m designers. >> >> I continue to disagree with this reasoning. I'm afraid I'm not really >> willing to allow widening the badness, unless altp2m was formally >> documented security-unsupported. > > Going the DOMCTL route here would have been the (much easier) solution, > and in fact, as stated before, there has been an attempt to do so - > however, IIRC Andrew has insisted that we should take care to use > consistent access privilege across altp2m operations. > > This was followed by a lengthy xen-devel discussion and several > unsuccessful attempts to obtain an official position from the original > contributors, at which point (after several months), as also discussed > at the Xen Developer Summit in Budapest, we decided to press on in the > direction that had seemed the most compatible with the original altp2m > design. (Please correct me if I'm misremembering or misunderstanding > something.) > > So at this point it looks like we're stuck again: we're happy to go in > any direction the maintainers decide is the best, but we do need to > decide on one. > > FWIW, Tamas (CC added) has added code to restrict where altp2m calls can > come from (although that's not XSM code). > > Please let us know how to proceed. I personally don't see anything wrong adding this as an HVMOP. The original idea with altp2m was that it will be used with VMFUNC where there is a guest kernel-level component that is trusted and coordinates with the hypervisor. With the setting I've added to the altp2m config it is now possible to limit whether the guest actually has the right to use the HVMOP or not, even without XSM. So adding this doesn't really mean its "widening the badness", it's configurable and could very well be appropriate depending on the use-case. Tamas ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable test] 116952: tolerable FAIL - PUSHED
flight 116952 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/116952/ Failures :-/ but no regressions. Regressions which are regarded as allowable (not blocking): test-armhf-armhf-xl-rtds16 guest-start/debian.repeat fail REGR. vs. 116891 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail blocked in 116891 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 116891 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail like 116891 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 116891 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 116891 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 116891 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 116891 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 116891 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 116891 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 116891 test-amd64-amd64-xl-pvhv2-amd 12 guest-start fail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qcow2 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-armhf-armhf-xl-xsm 13 migrate-support-checkfail never pass test-armhf-armhf-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass version targeted for testing: xen a04458bbf99f8fa64d727342938735727685f093 baseline version: xen 289adc1c56562d88e50b04245cd2027df8813bf4 Last test of basis 116891 2017-12-06 00:47:37 Z2 days Failing since116929 2017-12-06 22:30:06 Z1 days2 attempts Testing same since 116952 2017-12-07 19:01:53 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Jan Beulich jobs: build-amd64-xsm pass build-armhf-xsm pass build-i386-xsm pass build-amd64-xtf pass build-amd64 pass build-armhf pass build-i386 pass build-amd64-libvirt
[Xen-devel] [linux-4.9 test] 116954: regressions - FAIL
flight 116954 linux-4.9 real [real] http://logs.test-lab.xenproject.org/osstest/logs/116954/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-qemut-win7-amd64 16 guest-localmigrate/x10 fail in 116861 REGR. vs. 116754 Tests which are failing intermittently (not blocking): test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-localmigrate/x10 fail in 116861 pass in 116954 test-amd64-amd64-xl-qemut-win7-amd64 10 windows-installfail pass in 116861 test-amd64-amd64-xl-qemut-ws16-amd64 16 guest-localmigrate/x10 fail pass in 116931 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail in 116931 never pass test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 116754 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 116754 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 116754 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 116754 test-amd64-amd64-xl-pvhv2-amd 12 guest-start fail never pass test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qcow2 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-xsm 13 migrate-support-checkfail never pass test-armhf-armhf-xl-xsm 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass version targeted for testing: linux284bbc782445283e9a5124666dda8010f379f179 baseline version: linux8743ce3d7c9698285310920c443c086e337aef44 Last test of basis 116754 2017-12-01 16:36:51 Z7 days Testing same since 116861 2017-12-05 11:08:00 Z3 days4 attempts People who touched revisions under test: Adam Ford Adrian Hunter Alex Deucher Andrew Morton Anna Schumaker Bartosz Golaszewski Bastian Stender Ben Hutchings chenjie Christian König Christoph Hellwig Dan Carpenter Dan Williams David Sterba Dr. David Alan Gilbert Greg Kroah-Hartman Guenter Roeck guoxuenan Hans de Goede Heiner Kallweit Huacai Chen J.
[Xen-devel] [linux-linus test] 116947: regressions - FAIL
flight 116947 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/116947/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 115643 test-amd64-amd64-libvirt-pair 10 xen-boot/src_host fail REGR. vs. 115643 test-amd64-amd64-libvirt-pair 11 xen-boot/dst_host fail REGR. vs. 115643 test-amd64-amd64-pygrub 7 xen-boot fail REGR. vs. 115643 test-amd64-i386-pair 10 xen-boot/src_hostfail REGR. vs. 115643 test-amd64-i386-xl-qemut-debianhvm-amd64 7 xen-boot fail REGR. vs. 115643 test-amd64-amd64-xl-qemuu-ovmf-amd64 7 xen-boot fail REGR. vs. 115643 test-amd64-amd64-xl-qemuu-win10-i386 7 xen-boot fail REGR. vs. 115643 test-amd64-amd64-xl-pvhv2-amd 7 xen-bootfail REGR. vs. 115643 test-amd64-i386-xl-qemuu-debianhvm-amd64 7 xen-boot fail REGR. vs. 115643 test-amd64-i386-libvirt-xsm 7 xen-boot fail REGR. vs. 115643 test-amd64-i386-libvirt 7 xen-boot fail REGR. vs. 115643 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 115643 test-amd64-i386-xl-raw7 xen-boot fail REGR. vs. 115643 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 115643 test-amd64-i386-pair 11 xen-boot/dst_hostfail REGR. vs. 115643 test-amd64-i386-xl-xsm7 xen-boot fail REGR. vs. 115643 test-amd64-i386-freebsd10-i386 7 xen-boot fail REGR. vs. 115643 test-amd64-amd64-xl-qcow2 7 xen-boot fail REGR. vs. 115643 test-amd64-i386-freebsd10-amd64 7 xen-boot fail REGR. vs. 115643 test-amd64-amd64-xl-xsm 7 xen-boot fail REGR. vs. 115643 test-amd64-i386-xl-qemuu-ovmf-amd64 7 xen-boot fail REGR. vs. 115643 test-amd64-amd64-xl-multivcpu 7 xen-bootfail REGR. vs. 115643 test-amd64-i386-rumprun-i386 7 xen-boot fail REGR. vs. 115643 test-amd64-amd64-amd64-pvgrub 7 xen-bootfail REGR. vs. 115643 test-amd64-i386-xl7 xen-boot fail REGR. vs. 115643 test-amd64-i386-libvirt-qcow2 7 xen-bootfail REGR. vs. 115643 test-amd64-i386-examine 8 reboot fail REGR. vs. 115643 test-amd64-i386-qemut-rhel6hvm-amd 7 xen-boot fail REGR. vs. 115643 test-amd64-i386-qemuu-rhel6hvm-amd 7 xen-boot fail REGR. vs. 115643 test-amd64-amd64-xl 7 xen-boot fail REGR. vs. 115643 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 115643 test-amd64-amd64-xl-qemut-win7-amd64 7 xen-boot fail REGR. vs. 115643 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 115643 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 115643 test-amd64-amd64-libvirt-vhd 7 xen-boot fail REGR. vs. 115643 test-amd64-amd64-libvirt-xsm 7 xen-boot fail REGR. vs. 115643 test-amd64-amd64-xl-credit2 7 xen-boot fail REGR. vs. 115643 test-amd64-amd64-i386-pvgrub 7 xen-boot fail REGR. vs. 115643 test-amd64-amd64-rumprun-amd64 7 xen-boot fail REGR. vs. 115643 test-amd64-i386-libvirt-pair 10 xen-boot/src_hostfail REGR. vs. 115643 test-amd64-i386-libvirt-pair 11 xen-boot/dst_hostfail REGR. vs. 115643 test-amd64-amd64-xl-qemut-debianhvm-amd64 7 xen-bootfail REGR. vs. 115643 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 115643 test-amd64-amd64-pair10 xen-boot/src_hostfail REGR. vs. 115643 test-amd64-amd64-pair11 xen-boot/dst_hostfail REGR. vs. 115643 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 115643 test-armhf-armhf-libvirt-raw 6 xen-install fail REGR. vs. 115643 test-amd64-amd64-examine 8 reboot fail REGR. vs. 115643 Regressions which are regarded as allowable (not blocking): test-amd64-amd64-xl-rtds 7 xen-boot fail REGR. vs. 115643 Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail like 115643 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 115643 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 115643 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 115643 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 115643 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 115643 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 115643 test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-x
[Xen-devel] [libvirt test] 116965: tolerable all pass - PUSHED
flight 116965 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/116965/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 116935 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 116935 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail like 116935 test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-arm64-arm64-libvirt-qcow2 12 migrate-support-checkfail never pass test-arm64-arm64-libvirt-qcow2 13 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail never pass version targeted for testing: libvirt e2ad8e5993df3cf1ebb575cd960f8f3d0efd2424 baseline version: libvirt 7c7ec17738ad97faa302071a9cc8cad7ba02c2ea Last test of basis 116935 2017-12-07 04:20:39 Z1 days Testing same since 116965 2017-12-08 04:20:53 Z0 days1 attempts People who touched revisions under test: Chen Hanxiao Daniel P. Berrange Ján Tomko Michal Privoznik Peter Krempa jobs: build-amd64-xsm pass build-arm64-xsm pass build-armhf-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-arm64-libvirt pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-arm64-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-arm64-arm64-libvirt-xsm pass test-armhf-armhf-libvirt-xsm pass test-amd64-i386-libvirt-xsm pass test-amd64-amd64-libvirt pass test-arm64-arm64-libvirt pass test-armhf-armhf-libvirt pass test-amd64-i386-libvirt pass test-amd64-amd64-libvirt-pairpass test-amd64-i386-libvirt-pair pass test-arm64-arm64-libvirt-qcow2 pass test-armhf-armhf-libvirt-raw pass test-amd64-amd64-libvirt-vhd pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=
[Xen-devel] [seabios test] 116958: regressions - FAIL
flight 116958 seabios real [real] http://logs.test-lab.xenproject.org/osstest/logs/116958/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail REGR. vs. 115539 Tests which are failing intermittently (not blocking): test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-localmigrate/x10 fail pass in 116937 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail in 116937 like 115539 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 115539 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 115539 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass version targeted for testing: seabios df46d10c8a7b88eb82f3ceb2aa31782dee15593d baseline version: seabios 0ca6d6277dfafc671a5b3718cbeb5c78e2a888ea Last test of basis 115539 2017-11-03 20:48:58 Z 34 days Failing since115733 2017-11-10 17:19:59 Z 28 days 45 attempts Testing same since 116211 2017-11-16 00:20:45 Z 22 days 35 attempts People who touched revisions under test: Kevin O'Connor Stefan Berger 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-libvirt-qemuu-debianhvm-amd64-xsm pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsmpass test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm pass test-amd64-amd64-qemuu-nested-amdfail test-amd64-i386-qemuu-rhel6hvm-amd pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-i386-xl-qemuu-debianhvm-amd64 pass test-amd64-amd64-xl-qemuu-win7-amd64 fail test-amd64-i386-xl-qemuu-win7-amd64 fail test-amd64-amd64-xl-qemuu-ws16-amd64 fail test-amd64-i386-xl-qemuu-ws16-amd64 fail test-amd64-amd64-xl-qemuu-win10-i386 fail test-amd64-i386-xl-qemuu-win10-i386 fail test-amd64-amd64-qemuu-nested-intel pass test-amd64-i386-qemuu-rhel6hvm-intel 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 Not pushing. commit df46d10c8a7b88eb82f3ceb2aa31782dee15593d Author: Stefan Berger Date: Tue Nov 14 15:03:47 2017 -0500 tpm: Add support for TPM2 ACPI table Add support for the TPM2 ACPI table. If we find it and its of the appropriate size, we can get the log_area_start_address and log_area_minimum_size from it. The latest version of the spec can be found here: https://trustedcomputinggroup.org/tcg-acpi-specification/ Signed-off-by: Stefan Berger commit 0541f2f0f246e77d7c726926976920e8072d1119 Author: Kevin O'Connor Date: Fri Nov 10 12:20:35 2017 -0500 paravirt: Only enable sercon in NOGRAPHIC mode if no other console specified Signed-off-by: Kevin O'Connor commit 9ce6778f08c632c52b25bc8f754291ef18710d53 Author: Kevin O'Connor Date: Fri Nov 10 12:16:36 2017 -0500 docs: Add sercon-port to Runtime_config.md documentation Signed-off-by: Kevin O'Connor commit 63451fca13c75870e1703eb3e20584d9
Re: [Xen-devel] [RFC PATCH v2 1/2] xen/pvh: Add memory map pointer to hvm_start_info struct
Thanks for taking a look Jan. More below... On 12/8/2017 12:49 AM, Jan Beulich wrote: On 07.12.17 at 23:45, wrote: The start info structure that is defined as part of the x86/HVM direct boot ABI and used for starting Xen PVH guests would be more versatile if it also included a way to efficiently pass information about the memory map to the guest. That way Xen PVH guests would not be forced to use a hypercall to get the information and would make it easier for KVM guests to share the PVH entry point. --- include/xen/interface/hvm/start_info.h | 34 +++--- 1 file changed, 31 insertions(+), 3 deletions(-) First of all such a change should be submitted against the canonical copy of the header, which lives in the Xen tree. Understood. Will do that when this converts from RFC to actual patch. The argument of avoiding a hypercall doesn't really count imo - this isn't in any way performance critical code. The argument of making re-use easier is fine, though. Okay, I will reword the commit message. --- a/include/xen/interface/hvm/start_info.h +++ b/include/xen/interface/hvm/start_info.h @@ -33,7 +33,7 @@ *| magic | Contains the magic value XEN_HVM_START_MAGIC_VALUE *|| ("xEn3" with the 0x80 bit of the "E" set). * 4 ++ - *| version| Version of this structure. Current version is 0. New + *| version| Version of this structure. Current version is 1. New *|| versions are guaranteed to be backwards-compatible. * 8 ++ *| flags | SIF_xxx flags. @@ -48,6 +48,12 @@ * 32 ++ *| rsdp_paddr | Physical address of the RSDP ACPI data structure. * 40 ++ + *| memmap_paddr | Physical address of the memory map. Only present in + *|| version 1 and newer of the structure. + * 48 ++ + *| memmap_entries | Number of entries in the memory map table. Only + *|| present in version 1 and newer of the structure. + * 52 ++ Please let's make this optional even in v1 (and later), i.e. spell out that it may be zero. That way Xen code could continue to use the hypercall approach even. Yes, my intention was to make this optional. I will spell it out. Also please spell out a 4-byte reserved entry at the end, to make the specified structure a multiple of 8 in size again regardless of bitness of the producer/consumer. Sure, I can add that. @@ -62,6 +68,17 @@ *| reserved | * 32 ++ * + * The layout of each entry in the memory map table is as follows and no + * padding is used between entries in the array: + * + * 0 ++ + *| addr | Base address + * 8 ++ + *| size | Size of mapping + * 16 ++ + *| type | E820_TYPE_xxx + * 20 +| I'm not convinced of re-using E820 types here. I can see that this might ease the consumption in Linux, but I don't think there should be any connection to x86 aspects here - the data being supplied is x86-agnostic, and Linux'es placement of the header is also making no connection to x86 (oddly enough, the current placement in the Xen tree does, for a reason which escapes me). I could also imagine reasons to add new types without them being sanctioned by whoever maintains E820 type assignments. So there are three aspects to discuss here. 1) The addition of the "E820_TYPE_xxx" comment. I am fine with just changing that to "mapping type" and leaving it as something to be coordinated between the hypervisor and the guest OS being started by that hypervisor. 2) x86 vs x86-agnostic. While I'm trying to keep this interface generic in terms of guest OS (like Linux, FreeBSD, possible other guests in the future) and hypervisor type (Xen, QEMU/KVM, etc), I was actually under the impression that we are dealing with an ABI that is very much x86 specific. The canonical document describing the ABI (https://xenbits.xen.org/docs/unstable/misc/pvh.html) is titled "x86/HVM direct boot ABI" and goes on to describe an interface in very x86-specific terms. i.e. The ebx register must contain a pointer, cs, ds, es must be set a certain way, etc. That is probably why Xen's placement of the header file is in a x86 section of the tree. And also why there already exist a number of "x86" references in the existing header file. A quick grep of the existing header file will show lines like: "C representation of the x86/HVM start info layout" "Start of day structure passed to PVH guests and to HVM guests in %ebx" "Xen on x86 will always try to place all the data below the 4GiB" If at some point in the future someone decides to implement a similar ABI for a different CPU architecture while re-using this same hvm_start_info struct, then this header will have to be redon
Re: [Xen-devel] [RFC PATCH v2 0/2] KVM: x86: Allow Qemu/KVM to use PVH entry point
On 12/07/2017 05:45 PM, Maran Wilson wrote: > > Juergen also had a suggestion to split the different hypervisor types > early and use a common set of service functions instead of special casing > xen_guest everywhere. > > There are certainly less special cases in this version of the patch, but > if we still think it's important to split things up between common, Xen, > and KVM components, then I would appreciate a suggestion on how best that > can be done. Are we talking about just re-factoring functions in the > existing file? Or do we need to go all the way and pull all the PVH entry > code out of xen directories and find a home for it somewhere else so that > we can use kernels built without CONFIG_XEN to start KVM guests via the > PVH entry point. If the latter, any suggestions for which common files or > directories I can move this stuff to? I wonder whether the time has come for arch/x86/virt/ xen/ kvm/ hyperv/ kernel/paravirt* kernel/cpu/hypervisor.c -boris ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH V3 1/2] Drivers/PCI: Export pcie_has_flr() interface
On Thu, Dec 07, 2017 at 05:21:44PM -0500, Govinda Tatti wrote: > This patch exports pcie_has_flr() and it is being used by Xen pciback > driver to reset (flr/slot/bus) PCI devices based on 'reset' SysFS > attribute. > > Signed-off-by: Govinda Tatti > --- > v3: -New > > drivers/pci/pci.c | 3 ++- > include/linux/pci.h | 1 + > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index 6078dfc..499e922 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -3872,7 +3872,7 @@ static void pci_flr_wait(struct pci_dev *dev) > * Returns true if the device advertises support for PCIe function level > * resets. > */ > -static bool pcie_has_flr(struct pci_dev *dev) > +bool pcie_has_flr(struct pci_dev *dev) > { > u32 cap; > > @@ -3882,6 +3882,7 @@ static bool pcie_has_flr(struct pci_dev *dev) > pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &cap); > return cap & PCI_EXP_DEVCAP_FLR; > } > +EXPORT_SYMBOL_GPL(pcie_has_flr); I'd rather change pcie_flr() so you could *always* call it, and it would return 0, -ENOTTY, or whatever, based on whether FLR is supported. Is that feasible? I don't like the "Can I do this? Ok, do this" style of interfaces. It's racy (not really applicable in this case) and seems clunky. > /** > * pcie_flr - initiate a PCIe function level reset > diff --git a/include/linux/pci.h b/include/linux/pci.h > index d16a7c0..44bf2b5 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -1089,6 +1089,7 @@ int pcie_get_mps(struct pci_dev *dev); > int pcie_set_mps(struct pci_dev *dev, int mps); > int pcie_get_minimum_link(struct pci_dev *dev, enum pci_bus_speed *speed, > enum pcie_link_width *width); > +bool pcie_has_flr(struct pci_dev *dev); > void pcie_flr(struct pci_dev *dev); > int __pci_reset_function(struct pci_dev *dev); > int __pci_reset_function_locked(struct pci_dev *dev); > -- > 2.9.5 > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-next 06/16] xen/arm: Extend copy_to_guest to support copying from/to guest physical address
On Fri, 8 Dec 2017, Julien Grall wrote: > Hi Stefano, > > On 07/12/17 23:01, Stefano Stabellini wrote: > > On Wed, 6 Dec 2017, Julien Grall wrote: > > > Hi Stefano, > > > > > > On 12/06/2017 01:22 AM, Stefano Stabellini wrote: > > > > On Thu, 23 Nov 2017, Julien Grall wrote: > > > > > The only differences between copy_to_guest and > > > > > access_guest_memory_by_ipa > > > > > are: > > > > > - The latter does not support copying data crossing page > > > > > boundary > > > > > - The former is copying from/to guest VA whilst the latter from > > > > > guest PA > > > > > > > > > > copy_to_guest can easily be extended to support copying from/to guest > > > > > physical address. For that a new bit is used to tell whether linear > > > > > address or ipa is been used. > > > > > > > > > > Lastly access_guest_memory_by_ipa is reimplemented using > > > > > copy_to_guest. > > > > > This also has the benefits to extend the use of it, it is now possible > > > > > to copy data crossing page boundary. > > > > > > > > > > Signed-off-by: Julien Grall > > > > > > > > Ah! This is the reason why previous patches were not using vaddr_t. It > > > > makes sense now. May I suggest we use something different from paddr_t > > > > in copy_guest for addr type? I don't think is correct to specify addr as > > > > paddr_t when it could be vaddr_t; in the future we could have type > > > > checks on them. > > > > > > > > I suggest we specify it as u64, but if you have a better idea go for it. > > > > > > We should not use more u64 in the code. uint64_t could be a solution but > > > even > > > that, I don't see the reason. How are you sure the physical address will > > > always fit in 64-bit? > > > > > > On the other side, very likely vaddr_t will fit in paddr_t. So paddr_t is > > > the > > > right way to go for me. > > > > What about introducing xaddr_t? > > I would prefer uint64_t in that case. xaddr_t is quite confusing to read and > could be misused. > > > Or at least: > > > >static struct page_info *translate_get_page(struct vcpu *v, paddr_t /*or > > vaddr_t */ addr > > I can do that as well. What's your preference? I prefer uint64_t ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/arm: gic-v3: Bail out if gicv3_cpu_init fail
On Fri, 8 Dec 2017, Julien Grall wrote: > Hi Stefano, > > On 07/12/17 23:05, Stefano Stabellini wrote: > > On Wed, 6 Dec 2017, Julien Grall wrote: > > > From: Julien Grall > > > > > > When system registers are not enabled, all the access to them will trap > > ^ accesses > > > > > > > in EL2. In Xen, system registers will be enabled by gicv3_cpu_init only > > > on success. As the rest of the code (e.g gicv3_hyp_init) relies on > > > system register, it is better to bail out directly. > > > > > > This will save time on debugging early boot issue on GICv3 platform. > > > > > > Signed-off-by: Julien Grall > > > > This is good: > > > > Reviewed-by: Stefano Stabellini > > > > > > Do we also want to print a warning or an error message? > > AFAICT, all the path that return an error in gicv3_cpu_init (and the callers) > have already a warning/error message. So no need to add an extra one here. committed ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/arm: Surround HSR_SYSREG macro value with ()
On Fri, 8 Dec 2017, Julien Grall wrote: > Hi, > > Ping? > > Cheers, > > On 29/11/17 17:46, Julien Grall wrote: > > The value of the macro HCR_SYSREG is not surrounded by (). This means > > the behavior may change depend on how it is used. > > > > Thanksfully recent GCC will issue a warning for that. > > > > Signed-off-by: Julien Grall reviewed and committed > > --- > > > > I am not aware of any "bad" usage today in Xen. This was found whilst > > playing with sysreg emulation and GCC complaining about the missing (). > > --- > > xen/include/asm-arm/arm64/sysregs.h | 10 +- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/xen/include/asm-arm/arm64/sysregs.h > > b/xen/include/asm-arm/arm64/sysregs.h > > index 084d2a1e5d..1811234249 100644 > > --- a/xen/include/asm-arm/arm64/sysregs.h > > +++ b/xen/include/asm-arm/arm64/sysregs.h > > @@ -32,11 +32,11 @@ > > /* These are used to decode traps with HSR.EC==HSR_EC_SYSREG */ > > #define HSR_SYSREG(op0,op1,crn,crm,op2) \ > > -((__HSR_SYSREG_##op0) << HSR_SYSREG_OP0_SHIFT) | \ > > -((__HSR_SYSREG_##op1) << HSR_SYSREG_OP1_SHIFT) | \ > > -((__HSR_SYSREG_##crn) << HSR_SYSREG_CRN_SHIFT) | \ > > -((__HSR_SYSREG_##crm) << HSR_SYSREG_CRM_SHIFT) | \ > > -((__HSR_SYSREG_##op2) << HSR_SYSREG_OP2_SHIFT) > > +(((__HSR_SYSREG_##op0) << HSR_SYSREG_OP0_SHIFT) | \ > > + ((__HSR_SYSREG_##op1) << HSR_SYSREG_OP1_SHIFT) | \ > > + ((__HSR_SYSREG_##crn) << HSR_SYSREG_CRN_SHIFT) | \ > > + ((__HSR_SYSREG_##crm) << HSR_SYSREG_CRM_SHIFT) | \ > > + ((__HSR_SYSREG_##op2) << HSR_SYSREG_OP2_SHIFT)) > > #define HSR_SYSREG_DCISW HSR_SYSREG(1,0,c7,c6,2) > > #define HSR_SYSREG_DCCSW HSR_SYSREG(1,0,c7,c10,2) > > > > -- > Julien Grall > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/arm: bootfdt: Use proper default for #address-cells and #size-cells
On Fri, 8 Dec 2017, Julien Grall wrote: > Hi, > > On 29/11/17 18:12, Stefano Stabellini wrote: > > On Wed, 29 Nov 2017, Julien Grall wrote: > > > Per the device-tree specific [1], when the property #address-cells > > > and #size-cells are not present, the default value should be resp. 1 > > > and 2. > > > > > > [1] > > > https://www.devicetree.org/downloads/devicetree-specification-v0.1-20160524.pdf > > > > > > Signed-off-by: Julien Grall > > > > Acked-by: Stefano Stabellini > > This was acked but not applied to staging. Can you do it please? It was in my arm-next branch together with a couple of patches from Andre. I committed all three to staging. > > > > > > > > --- > > > > > > This was discovered debugging DT generated by GRUB on ACPI-only > > > platform. I am not aware of any DT relying on that for now, but it > > > would still be nice to be compliant with the spec and avoid > > > surprise. > > > --- > > > xen/arch/arm/bootfdt.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c > > > index 4a687e725d..8eba42c7b9 100644 > > > --- a/xen/arch/arm/bootfdt.c > > > +++ b/xen/arch/arm/bootfdt.c > > > @@ -109,8 +109,8 @@ int __init device_tree_for_each_node(const void *fdt, > > > continue; > > > } > > > -as = depth > 0 ? address_cells[depth-1] : 0; > > > -ss = depth > 0 ? size_cells[depth-1] : 0; > > > +as = depth > 0 ? address_cells[depth-1] : > > > DT_ROOT_NODE_ADDR_CELLS_DEFAULT; > > > +ss = depth > 0 ? size_cells[depth-1] : > > > DT_ROOT_NODE_SIZE_CELLS_DEFAULT; > > > address_cells[depth] = device_tree_get_u32(fdt, node, > > > "#address-cells", > > > as); > > > -- > > > 2.11.0 > > > > > -- > Julien Grall > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 03/10] ARM: VGIC: move gic_remove_irq_from_queues()
On Thu, 7 Dec 2017, Andre Przywara wrote: > gic_remove_irq_from_queues() was not only misnamed, it also has the wrong > abstraction, as it should not live in gic.c. > Move it into vgic.c and vgic.h, where it belongs to, and rename it on > the way. > > Signed-off-by: Andre Przywara > Reviewed-by: Stefano Stabellini I committed the first three patches > --- > xen/arch/arm/gic.c | 9 - > xen/arch/arm/vgic-v3-its.c | 4 ++-- > xen/arch/arm/vgic.c| 11 ++- > xen/include/asm-arm/gic.h | 1 - > xen/include/asm-arm/vgic.h | 1 + > 5 files changed, 13 insertions(+), 13 deletions(-) > > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c > index ed363f6c37..bac8ada2bb 100644 > --- a/xen/arch/arm/gic.c > +++ b/xen/arch/arm/gic.c > @@ -411,15 +411,6 @@ void gic_remove_from_lr_pending(struct vcpu *v, struct > pending_irq *p) > list_del_init(&p->lr_queue); > } > > -void gic_remove_irq_from_queues(struct vcpu *v, struct pending_irq *p) > -{ > -ASSERT(spin_is_locked(&v->arch.vgic.lock)); > - > -clear_bit(GIC_IRQ_GUEST_QUEUED, &p->status); > -list_del_init(&p->inflight); > -gic_remove_from_lr_pending(v, p); > -} > - > void gic_raise_inflight_irq(struct vcpu *v, unsigned int virtual_irq) > { > struct pending_irq *n = irq_to_pending(v, virtual_irq); > diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c > index 72a5c70656..d8fa44258d 100644 > --- a/xen/arch/arm/vgic-v3-its.c > +++ b/xen/arch/arm/vgic-v3-its.c > @@ -381,7 +381,7 @@ static int its_handle_clear(struct virt_its *its, > uint64_t *cmdptr) > * have no active state, we don't need to care about this here. > */ > if ( !test_bit(GIC_IRQ_GUEST_VISIBLE, &p->status) ) > -gic_remove_irq_from_queues(vcpu, p); > +vgic_remove_irq_from_queues(vcpu, p); > > spin_unlock_irqrestore(&vcpu->arch.vgic.lock, flags); > ret = 0; > @@ -619,7 +619,7 @@ static int its_discard_event(struct virt_its *its, > } > > /* Cleanup the pending_irq and disconnect it from the LPI. */ > -gic_remove_irq_from_queues(vcpu, p); > +vgic_remove_irq_from_queues(vcpu, p); > vgic_init_pending_irq(p, INVALID_LPI); > > spin_unlock_irqrestore(&vcpu->arch.vgic.lock, flags); > diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c > index d8acbbeaaa..6e933a86d3 100644 > --- a/xen/arch/arm/vgic.c > +++ b/xen/arch/arm/vgic.c > @@ -281,7 +281,7 @@ bool vgic_migrate_irq(struct vcpu *old, struct vcpu *new, > unsigned int irq) > /* If the IRQ is still lr_pending, re-inject it to the new vcpu */ > if ( !list_empty(&p->lr_queue) ) > { > -gic_remove_irq_from_queues(old, p); > +vgic_remove_irq_from_queues(old, p); > irq_set_affinity(p->desc, cpumask_of(new->processor)); > spin_unlock_irqrestore(&old->arch.vgic.lock, flags); > vgic_vcpu_inject_irq(new, irq); > @@ -508,6 +508,15 @@ void vgic_clear_pending_irqs(struct vcpu *v) > spin_unlock_irqrestore(&v->arch.vgic.lock, flags); > } > > +void vgic_remove_irq_from_queues(struct vcpu *v, struct pending_irq *p) > +{ > +ASSERT(spin_is_locked(&v->arch.vgic.lock)); > + > +clear_bit(GIC_IRQ_GUEST_QUEUED, &p->status); > +list_del_init(&p->inflight); > +gic_remove_from_lr_pending(v, p); > +} > + > void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int virq) > { > uint8_t priority; > diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h > index d3d7bda50d..587a14f8b9 100644 > --- a/xen/include/asm-arm/gic.h > +++ b/xen/include/asm-arm/gic.h > @@ -244,7 +244,6 @@ extern void gic_raise_guest_irq(struct vcpu *v, unsigned > int irq, > unsigned int priority); > extern void gic_raise_inflight_irq(struct vcpu *v, unsigned int virtual_irq); > extern void gic_remove_from_lr_pending(struct vcpu *v, struct pending_irq > *p); > -extern void gic_remove_irq_from_queues(struct vcpu *v, struct pending_irq > *p); > > /* Accept an interrupt from the GIC and dispatch its handler */ > extern void gic_interrupt(struct cpu_user_regs *regs, int is_fiq); > diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h > index e489d0bf21..2a93a7bef9 100644 > --- a/xen/include/asm-arm/vgic.h > +++ b/xen/include/asm-arm/vgic.h > @@ -204,6 +204,7 @@ extern int vcpu_vgic_init(struct vcpu *v); > extern struct vcpu *vgic_get_target_vcpu(struct vcpu *v, unsigned int virq); > extern void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int virq); > extern void vgic_vcpu_inject_spi(struct domain *d, unsigned int virq); > +extern void vgic_remove_irq_from_queues(struct vcpu *v, struct pending_irq > *p); > extern void vgic_clear_pending_irqs(struct vcpu *v); > extern void vgic_init_pending_irq(struct pending_irq *p, unsigned int virq); > extern struct pending_irq *irq_to_pending(struct vcpu *v, unsigned int irq); > -- > 2.14.1 > ___ Xen-devel mailing list Xen-devel@lists.xenproje
Re: [Xen-devel] [PATCH 3/3] x86/PCI: limit the size of the 64bit BAR to 256GB
On 12/08/2017 12:56 PM, Bjorn Helgaas wrote: > On Wed, Dec 06, 2017 at 01:51:18PM -0600, Bjorn Helgaas wrote: >> On Wed, Nov 29, 2017 at 03:12:29PM +0100, Christian König wrote: >>> This avoids problems with Xen which hides some memory resources from the >>> OS and potentially also allows memory hotplug while this fixup is >>> enabled. >> The patch itself is OK, but the changelog doesn't say enough about >> what the problem is. I have no clue about what the Xen issue is or >> why limiting the BAR to 256GB avoids the problem or what this has to >> do with memory hotplug. >> >> For example, we should be able to tell why 256GB is the right number. >> Maybe there's something specific in Xen you can reference? Maybe an >> example of what goes wrong with some details? > Ping? Is this change required to fix issues people are seeing? If > so, we either need to rework the changelog and get it merged, or > revert the quirk as a whole. > > I tentatively applied the first two patches to for-linus, but I > haven't asked Linus to pull them because I assumed we really needed > all three. This is not a fix but rather is a workaround. The problem is that Xen dom0 may be running with less than all of the system memory and the chunk of host memory that dom0 doesn't have is not exposed in e820 as reserved. And so pci_amd_enable_64bit_bar() assumes that it can be used for MMIO, with predictable results. Only trying to use very high addresses limits chances that there is memory there. The alternative is to revert f5775e0b6116b7e2425ccf535243b21768566d87. I have been working on a proper fix but haven't been able to finish it yet. -boris > > Bjorn > >>> Signed-off-by: Christian König >>> --- >>> arch/x86/pci/fixup.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c >>> index c817ab85dc82..149adbc7f2a3 100644 >>> --- a/arch/x86/pci/fixup.c >>> +++ b/arch/x86/pci/fixup.c >>> @@ -701,7 +701,7 @@ static void pci_amd_enable_64bit_bar(struct pci_dev >>> *dev) >>> res->name = "PCI Bus :00"; >>> res->flags = IORESOURCE_PREFETCH | IORESOURCE_MEM | >>> IORESOURCE_MEM_64 | IORESOURCE_WINDOW; >>> - res->start = 0x1ull; >>> + res->start = 0xbdull; >>> res->end = 0xfdull - 1; >>> >>> /* Just grab the free area behind system memory for this */ >>> -- >>> 2.11.0 >>> ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 04/10] ARM: VGIC: streamline gic_restore_pending_irqs()
On Thu, 7 Dec 2017, Andre Przywara wrote: > In gic_restore_pending_irqs() we push our pending virtual IRQs into the > list registers. This function is called once from a GIC context and once > from a VGIC context. Refactor the calls so that we have only one callsite > from the VGIC context. This will help separating the two worlds later. > > Signed-off-by: Andre Przywara > --- > xen/arch/arm/domain.c | 1 + > xen/arch/arm/gic.c| 11 +-- > xen/arch/arm/traps.c | 2 +- > xen/include/asm-arm/gic.h | 2 +- > 4 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c > index a74ff1c07c..73f4d4b2b2 100644 > --- a/xen/arch/arm/domain.c > +++ b/xen/arch/arm/domain.c > @@ -185,6 +185,7 @@ static void ctxt_switch_to(struct vcpu *n) > > /* VGIC */ > gic_restore_state(n); > +gic_inject(n); > > /* VFP */ > vfp_restore_state(n); > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c > index bac8ada2bb..1f00654ef5 100644 > --- a/xen/arch/arm/gic.c > +++ b/xen/arch/arm/gic.c > @@ -36,8 +36,6 @@ > #include > #include > > -static void gic_restore_pending_irqs(struct vcpu *v); > - > static DEFINE_PER_CPU(uint64_t, lr_mask); > > #define lr_all_full() (this_cpu(lr_mask) == ((1 << gic_hw_ops->info->nr_lrs) > - 1)) > @@ -91,8 +89,6 @@ void gic_restore_state(struct vcpu *v) > gic_hw_ops->restore_state(v); > > isb(); > - > -gic_restore_pending_irqs(v); > } > > /* desc->irq needs to be disabled before calling this function */ > @@ -715,11 +711,14 @@ out: > return rc; > } > > -void gic_inject(void) > +void gic_inject(struct vcpu *v) > { > ASSERT(!local_irq_is_enabled()); > > -gic_restore_pending_irqs(current); > +gic_restore_pending_irqs(v); This looks suspicious to me: gic_restore_pending_irqs calls gic_set_lr. It doesn't look like gic_restore_pending_irqs can actually be called for v != current. However, I think that we could remove the call to gic_restore_pending_irqs from gic_restore_state safely, because we can still rely on gic_inject being called before entering the guest. There is no need to add a call to gic_inject in ctxt_switch_to, I think. > +if ( v != current ) > +return; > > if ( !list_empty(¤t->arch.vgic.lr_pending) && lr_all_full() ) > gic_hw_ops->update_hcr_status(GICH_HCR_UIE, true); > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c > index ff3d6ff2aa..7fd676ed9d 100644 > --- a/xen/arch/arm/traps.c > +++ b/xen/arch/arm/traps.c > @@ -2298,7 +2298,7 @@ void leave_hypervisor_tail(void) > { > local_irq_disable(); > if (!softirq_pending(smp_processor_id())) { > -gic_inject(); > +gic_inject(current); > > /* > * If the SErrors handle option is "DIVERSE", we have to prevent > diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h > index 587a14f8b9..28cf16654a 100644 > --- a/xen/include/asm-arm/gic.h > +++ b/xen/include/asm-arm/gic.h > @@ -235,7 +235,7 @@ extern int gic_route_irq_to_guest(struct domain *, > unsigned int virq, > int gic_remove_irq_from_guest(struct domain *d, unsigned int virq, >struct irq_desc *desc); > > -extern void gic_inject(void); > +extern void gic_inject(struct vcpu *v); > extern void gic_clear_pending_irqs(struct vcpu *v); > extern int gic_events_need_delivery(void); ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 02/10] ARM: vGIC: fix nr_irq definition
Hi, On 12/07/2017 04:14 PM, Andre Przywara wrote: The global variable "nr_irqs" is used for x86 and some common Xen code. To make the latter work easily for ARM, it was #defined to NR_IRQS. This not only violated the common habit of capitalizing macros, but also caused issues if one wanted to use a rather innocent "nr_irqs" as a local variable name or as a function parameter. Drop the optimization and make nr_irqs a normal variable for ARM also. Signed-off-by: Andre Przywara Reviewed-by: Stefano Stabellini Surprisingly the reviewed-by was not carried in the merge. Anyway... --- xen/arch/arm/irq.c| 2 ++ xen/include/asm-arm/irq.h | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c index cbc7e6ebb8..7f133de549 100644 --- a/xen/arch/arm/irq.c +++ b/xen/arch/arm/irq.c @@ -27,6 +27,8 @@ #include #include +unsigned int __read_mostly nr_irqs = NR_IRQS; I am ok with turning this to a variable. But this should really be const and not let a chance of someone to override it by mistake. Can you please send a follow-up to fix that. + static unsigned int local_irqs_type[NR_LOCAL_IRQS]; static DEFINE_SPINLOCK(local_irqs_type_lock); diff --git a/xen/include/asm-arm/irq.h b/xen/include/asm-arm/irq.h index 2de76d0f56..abc8f06a13 100644 --- a/xen/include/asm-arm/irq.h +++ b/xen/include/asm-arm/irq.h @@ -31,7 +31,7 @@ struct arch_irq_desc { /* LPIs are always numbered starting at 8192, so 0 is a good invalid case. */ #define INVALID_LPI 0 -#define nr_irqs NR_IRQS +extern unsigned int nr_irqs; #define nr_static_irqs NR_IRQS #define arch_hwdom_irqs(domid) NR_IRQS Cheers -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 05/10] ARM: VGIC: split gic.c to observe hardware/virtual GIC separation
On Thu, 7 Dec 2017, Andre Przywara wrote: > Currently gic.c holds code to handle hardware IRQs as well as code to > bridge VGIC requests to the GIC virtualization hardware. > Despite being named gic.c, this file reaches into the VGIC and uses data > structures describing virtual IRQs. > To improve abstraction, move the VGIC functions into a separate file, > so that gic.c does what is says on the tin. > > Signed-off-by: Andre Przywara Reviewed-by: Stefano Stabellini > --- > xen/arch/arm/Makefile | 1 + > xen/arch/arm/gic-vgic.c | 413 > > xen/arch/arm/gic.c | 366 +- > 3 files changed, 416 insertions(+), 364 deletions(-) > create mode 100644 xen/arch/arm/gic-vgic.c > > diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile > index 30a2a6500a..41d7366527 100644 > --- a/xen/arch/arm/Makefile > +++ b/xen/arch/arm/Makefile > @@ -16,6 +16,7 @@ obj-y += domain_build.o > obj-y += domctl.o > obj-$(EARLY_PRINTK) += early_printk.o > obj-y += gic.o > +obj-y += gic-vgic.o > obj-y += gic-v2.o > obj-$(CONFIG_HAS_GICV3) += gic-v3.o > obj-$(CONFIG_HAS_ITS) += gic-v3-its.o > diff --git a/xen/arch/arm/gic-vgic.c b/xen/arch/arm/gic-vgic.c > new file mode 100644 > index 00..971b3bfe37 > --- /dev/null > +++ b/xen/arch/arm/gic-vgic.c > @@ -0,0 +1,413 @@ > +/* > + * xen/arch/arm/gic-vgic.c > + * > + * ARM Generic Interrupt Controller virtualization support > + * > + * Tim Deegan > + * Copyright (c) 2011 Citrix Systems. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +extern uint64_t per_cpu__lr_mask; > +extern const struct gic_hw_operations *gic_hw_ops; > + > +#define lr_all_full() (this_cpu(lr_mask) == ((1 << gic_hw_ops->info->nr_lrs) > - 1)) > + > +#undef GIC_DEBUG > + > +static void gic_update_one_lr(struct vcpu *v, int i); > + > +static inline void gic_set_lr(int lr, struct pending_irq *p, > + unsigned int state) > +{ > +ASSERT(!local_irq_is_enabled()); > + > +clear_bit(GIC_IRQ_GUEST_PRISTINE_LPI, &p->status); > + > +gic_hw_ops->update_lr(lr, p, state); > + > +set_bit(GIC_IRQ_GUEST_VISIBLE, &p->status); > +clear_bit(GIC_IRQ_GUEST_QUEUED, &p->status); > +p->lr = lr; > +} > + > +static inline void gic_add_to_lr_pending(struct vcpu *v, struct pending_irq > *n) > +{ > +struct pending_irq *iter; > + > +ASSERT(spin_is_locked(&v->arch.vgic.lock)); > + > +if ( !list_empty(&n->lr_queue) ) > +return; > + > +list_for_each_entry ( iter, &v->arch.vgic.lr_pending, lr_queue ) > +{ > +if ( iter->priority > n->priority ) > +{ > +list_add_tail(&n->lr_queue, &iter->lr_queue); > +return; > +} > +} > +list_add_tail(&n->lr_queue, &v->arch.vgic.lr_pending); > +} > + > +void gic_remove_from_lr_pending(struct vcpu *v, struct pending_irq *p) > +{ > +ASSERT(spin_is_locked(&v->arch.vgic.lock)); > + > +list_del_init(&p->lr_queue); > +} > + > +void gic_raise_inflight_irq(struct vcpu *v, unsigned int virtual_irq) > +{ > +struct pending_irq *n = irq_to_pending(v, virtual_irq); > + > +/* If an LPI has been removed meanwhile, there is nothing left to raise. > */ > +if ( unlikely(!n) ) > +return; > + > +ASSERT(spin_is_locked(&v->arch.vgic.lock)); > + > +/* Don't try to update the LR if the interrupt is disabled */ > +if ( !test_bit(GIC_IRQ_GUEST_ENABLED, &n->status) ) > +return; > + > +if ( list_empty(&n->lr_queue) ) > +{ > +if ( v == current ) > +gic_update_one_lr(v, n->lr); > +} > +#ifdef GIC_DEBUG > +else > +gdprintk(XENLOG_DEBUG, "trying to inject irq=%u into d%dv%d, when it > is still lr_pending\n", > + virtual_irq, v->domain->domain_id, v->vcpu_id); > +#endif > +} > + > +/* > + * Find an unused LR to insert an IRQ into, starting with the LR given > + * by @lr. If this new interrupt is a PRISTINE LPI, scan the other LRs to > + * avoid inserting the same IRQ twice. This situation can occur when an > + * event gets discarded while the LPI is in an LR, and a new LPI with the > + * same number gets mapped quickly afterwards. > + */ > +static unsigned int gic_find_unus
Re: [Xen-devel] [PATCH v2 06/10] ARM: VGIC: split up gic_dump_info() to cover virtual part separately
On Thu, 7 Dec 2017, Andre Przywara wrote: > Currently gic_dump_info() not only dumps the hardware state of the GIC, > but also the VGIC internal virtual IRQ lists. > Split the latter off and move it into gic-vgic.c to observe the abstraction. > > Signed-off-by: Andre Przywara Reviewed-by: Stefano Stabellini > --- > xen/arch/arm/domain.c | 1 + > xen/arch/arm/gic-vgic.c | 11 +++ > xen/arch/arm/gic.c| 12 > xen/include/asm-arm/gic.h | 1 + > 4 files changed, 13 insertions(+), 12 deletions(-) > > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c > index 73f4d4b2b2..5d2943b800 100644 > --- a/xen/arch/arm/domain.c > +++ b/xen/arch/arm/domain.c > @@ -942,6 +942,7 @@ long arch_do_vcpu_op(int cmd, struct vcpu *v, > XEN_GUEST_HANDLE_PARAM(void) arg) > void arch_dump_vcpu_info(struct vcpu *v) > { > gic_dump_info(v); > +gic_dump_vgic_info(v); > } > > void vcpu_mark_events_pending(struct vcpu *v) > diff --git a/xen/arch/arm/gic-vgic.c b/xen/arch/arm/gic-vgic.c > index 971b3bfe37..90b827c574 100644 > --- a/xen/arch/arm/gic-vgic.c > +++ b/xen/arch/arm/gic-vgic.c > @@ -403,6 +403,17 @@ void gic_inject(struct vcpu *v) > gic_hw_ops->update_hcr_status(GICH_HCR_UIE, 1); > } > > +void gic_dump_vgic_info(struct vcpu *v) > +{ > +struct pending_irq *p; > + > +list_for_each_entry ( p, &v->arch.vgic.inflight_irqs, inflight ) > +printk("Inflight irq=%u lr=%u\n", p->irq, p->lr); > + > +list_for_each_entry( p, &v->arch.vgic.lr_pending, lr_queue ) > +printk("Pending irq=%d\n", p->irq); > +} > + > /* > * Local variables: > * mode: C > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c > index 04e6d66b69..4cb74d449e 100644 > --- a/xen/arch/arm/gic.c > +++ b/xen/arch/arm/gic.c > @@ -443,20 +443,8 @@ static void maintenance_interrupt(int irq, void *dev_id, > struct cpu_user_regs *r > > void gic_dump_info(struct vcpu *v) > { > -struct pending_irq *p; > - > printk("GICH_LRs (vcpu %d) mask=%"PRIx64"\n", v->vcpu_id, > v->arch.lr_mask); > gic_hw_ops->dump_state(v); > - > -list_for_each_entry ( p, &v->arch.vgic.inflight_irqs, inflight ) > -{ > -printk("Inflight irq=%u lr=%u\n", p->irq, p->lr); > -} > - > -list_for_each_entry( p, &v->arch.vgic.lr_pending, lr_queue ) > -{ > -printk("Pending irq=%d\n", p->irq); > -} > } > > void init_maintenance_interrupt(void) > diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h > index 28cf16654a..4f4fd555c1 100644 > --- a/xen/include/asm-arm/gic.h > +++ b/xen/include/asm-arm/gic.h > @@ -285,6 +285,7 @@ extern void send_SGI_allbutself(enum gic_sgi sgi); > > /* print useful debug info */ > extern void gic_dump_info(struct vcpu *v); > +extern void gic_dump_vgic_info(struct vcpu *v); > > /* Number of interrupt lines */ > extern unsigned int gic_number_lines(void); > -- > 2.14.1 > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 10/10] ARM: VGIC: rework gicv[23]_update_lr to not use pending_irq
On Thu, 7 Dec 2017, Andre Przywara wrote: > The functions to actually populate a list register were accessing > the VGIC internal pending_irq struct, although they should be abstracting > from that. > Break the needed information down to remove the reference to pending_irq > from gic-v[23].c. > > Signed-off-by: Andre Przywara Reviewed-by: Stefano Stabellini > --- > xen/arch/arm/gic-v2.c | 14 +++--- > xen/arch/arm/gic-v3.c | 12 ++-- > xen/arch/arm/gic-vgic.c | 3 ++- > xen/include/asm-arm/gic.h | 4 ++-- > xen/include/asm-arm/irq.h | 3 +++ > 5 files changed, 20 insertions(+), 16 deletions(-) > > diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c > index 511c8d7294..2b271ba322 100644 > --- a/xen/arch/arm/gic-v2.c > +++ b/xen/arch/arm/gic-v2.c > @@ -428,8 +428,8 @@ static void gicv2_disable_interface(void) > spin_unlock(&gicv2.lock); > } > > -static void gicv2_update_lr(int lr, const struct pending_irq *p, > -unsigned int state) > +static void gicv2_update_lr(int lr, unsigned int virq, uint8_t priority, > +unsigned int hw_irq, unsigned int state) > { > uint32_t lr_reg; > > @@ -437,12 +437,12 @@ static void gicv2_update_lr(int lr, const struct > pending_irq *p, > BUG_ON(lr < 0); > > lr_reg = (((state & GICH_V2_LR_STATE_MASK) << GICH_V2_LR_STATE_SHIFT) | > - ((GIC_PRI_TO_GUEST(p->priority) & GICH_V2_LR_PRIORITY_MASK) > - << GICH_V2_LR_PRIORITY_SHIFT) | > - ((p->irq & GICH_V2_LR_VIRTUAL_MASK) << > GICH_V2_LR_VIRTUAL_SHIFT)); > + ((GIC_PRI_TO_GUEST(priority) & GICH_V2_LR_PRIORITY_MASK) > + << GICH_V2_LR_PRIORITY_SHIFT) | > + ((virq & GICH_V2_LR_VIRTUAL_MASK) << > GICH_V2_LR_VIRTUAL_SHIFT)); > > -if ( p->desc != NULL ) > -lr_reg |= GICH_V2_LR_HW | ((p->desc->irq & GICH_V2_LR_PHYSICAL_MASK ) > +if ( hw_irq != INVALID_IRQ ) > +lr_reg |= GICH_V2_LR_HW | ((hw_irq & GICH_V2_LR_PHYSICAL_MASK ) > << GICH_V2_LR_PHYSICAL_SHIFT); > > writel_gich(lr_reg, GICH_LR + lr * 4); > diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c > index 473e26111f..ce1e5cad25 100644 > --- a/xen/arch/arm/gic-v3.c > +++ b/xen/arch/arm/gic-v3.c > @@ -962,8 +962,8 @@ static void gicv3_disable_interface(void) > spin_unlock(&gicv3.lock); > } > > -static void gicv3_update_lr(int lr, const struct pending_irq *p, > -unsigned int state) > +static void gicv3_update_lr(int lr, unsigned int virq, uint8_t priority, > +unsigned int hw_irq, unsigned int state) > { > uint64_t val = 0; > > @@ -979,11 +979,11 @@ static void gicv3_update_lr(int lr, const struct > pending_irq *p, > if ( current->domain->arch.vgic.version == GIC_V3 ) > val |= GICH_LR_GRP1; > > -val |= ((uint64_t)p->priority & 0xff) << GICH_LR_PRIORITY_SHIFT; > -val |= ((uint64_t)p->irq & GICH_LR_VIRTUAL_MASK) << > GICH_LR_VIRTUAL_SHIFT; > +val |= (uint64_t)priority << GICH_LR_PRIORITY_SHIFT; > +val |= ((uint64_t)virq & GICH_LR_VIRTUAL_MASK) << GICH_LR_VIRTUAL_SHIFT; > > - if ( p->desc != NULL ) > - val |= GICH_LR_HW | (((uint64_t)p->desc->irq & GICH_LR_PHYSICAL_MASK) > + if ( hw_irq != INVALID_IRQ ) > + val |= GICH_LR_HW | (((uint64_t)hw_irq & GICH_LR_PHYSICAL_MASK) > << GICH_LR_PHYSICAL_SHIFT); > > gicv3_ich_write_lr(lr, val); > diff --git a/xen/arch/arm/gic-vgic.c b/xen/arch/arm/gic-vgic.c > index 8d43a6ba76..60f6498092 100644 > --- a/xen/arch/arm/gic-vgic.c > +++ b/xen/arch/arm/gic-vgic.c > @@ -52,7 +52,8 @@ static inline void gic_set_lr(int lr, struct pending_irq *p, > > clear_bit(GIC_IRQ_GUEST_PRISTINE_LPI, &p->status); > > -gic_hw_ops->update_lr(lr, p, state); > +gic_hw_ops->update_lr(lr, p->irq, p->priority, > + p->desc ? p->desc->irq : INVALID_IRQ, state); > > set_bit(GIC_IRQ_GUEST_VISIBLE, &p->status); > clear_bit(GIC_IRQ_GUEST_QUEUED, &p->status); > diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h > index 4f4fd555c1..ce9d1d058a 100644 > --- a/xen/include/asm-arm/gic.h > +++ b/xen/include/asm-arm/gic.h > @@ -342,8 +342,8 @@ struct gic_hw_operations { > /* Disable CPU physical and virtual interfaces */ > void (*disable_interface)(void); > /* Update LR register with state and priority */ > -void (*update_lr)(int lr, const struct pending_irq *pending_irq, > - unsigned int state); > +void (*update_lr)(int lr, unsigned int virq, uint8_t priority, > + unsigned int hw_irq, unsigned int state); > /* Update HCR status register */ > void (*update_hcr_status)(uint32_t flag, bool set); > /* Clear LR register */ > diff --git a/xen/include/asm-arm/irq.h b/xe
Re: [Xen-devel] [PATCH for-next 07/16] xen/arm: Introduce copy_to_guest_phys_flush_dcache
On Fri, 8 Dec 2017, Julien Grall wrote: > On 06/12/17 12:27, Julien Grall wrote: > > On 12/06/2017 01:26 AM, Stefano Stabellini wrote: > > > On Thu, 23 Nov 2017, Julien Grall wrote: > > > > Hi Andrew, > > > > > > > > On 23/11/17 18:49, Andrew Cooper wrote: > > > > > On 23/11/17 18:32, Julien Grall wrote: > > > > > > This new function will be used in a follow-up patch to copy data to > > > > > > the > > > > > > guest > > > > > > using the IPA (aka guest physical address) and then clean the cache. > > > > > > > > > > > > Signed-off-by: Julien Grall > > > > > > --- > > > > > > xen/arch/arm/guestcopy.c | 10 ++ > > > > > > xen/include/asm-arm/guest_access.h | 6 ++ > > > > > > 2 files changed, 16 insertions(+) > > > > > > > > > > > > diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c > > > > > > index be53bee559..7958663970 100644 > > > > > > --- a/xen/arch/arm/guestcopy.c > > > > > > +++ b/xen/arch/arm/guestcopy.c > > > > > > @@ -110,6 +110,16 @@ unsigned long raw_copy_from_guest(void *to, > > > > > > const > > > > > > void __user *from, unsigned le > > > > > > COPY_from_guest | COPY_linear); > > > > > > } > > > > > > +unsigned long copy_to_guest_phys_flush_dcache(struct domain *d, > > > > > > + paddr_t gpa, > > > > > > + void *buf, > > > > > > + unsigned int len) > > > > > > +{ > > > > > > + /* P2M is shared between all vCPUs, so the vCPU used does not > > > > > > matter. > > > > > > */ > > > > > > > > > > Be very careful with this line of thinking. It is only works after > > > > > DOMCTL_max_vcpus has succeeded, and before that point, it is a latent > > > > > NULL pointer dereference. > > > > > > > > I really don't expect that function been used before DOMCT_max_vcpus is > > > > set. > > > > It is only used for hardware emulation or Xen loading image into the > > > > hardware > > > > domain memory. I could add a check d->vcpus to be safe. > > > > > > > > > > > > > > Also, what about vcpus configured with alternative views? > > > > > > > > It is not important because the underlying call is get_page_from_gfn > > > > that does > > > > not care about the alternative view (that function take a domain in > > > > parameter). I can update the comment. > > > Since this is a new function, would it make sense to take a struct > > > vcpu* as parameter, instead of a struct domain* ? > > > > Well, I suggested this patch this way because likely everyone will use with > > d->vcpus[0]. And then you would have to wonder why d->vcpus[0] and not > > d->vcpus[1]... > > Thinking a bit more to this, it might be better/safer to pass either a domain > or a vCPU to copy_guest. I can see 2 solutions: > 1# Introduce a union that use the same parameter: > union > { > struct > { > struct domain *d; > } ipa; > struct > { > struct vcpu *v; > } gva; > } > The structure here would be to ensure that it is clear that only > domain (resp. vcpu) should be used with ipa (resp. gva). > > 2# Have 2 parameters, vcpu and domain. > > Any opinions? I think that would be clearer. You could also add a paddr_t/vaddr_t correspondingly inside the union maybe.___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-next 07/16] xen/arm: Introduce copy_to_guest_phys_flush_dcache
On 8 Dec 2017 22:26, "Stefano Stabellini" wrote: On Fri, 8 Dec 2017, Julien Grall wrote: > On 06/12/17 12:27, Julien Grall wrote: > > On 12/06/2017 01:26 AM, Stefano Stabellini wrote: > > > On Thu, 23 Nov 2017, Julien Grall wrote: > > > > Hi Andrew, > > > > > > > > On 23/11/17 18:49, Andrew Cooper wrote: > > > > > On 23/11/17 18:32, Julien Grall wrote: > > > > > > This new function will be used in a follow-up patch to copy data to > > > > > > the > > > > > > guest > > > > > > using the IPA (aka guest physical address) and then clean the cache. > > > > > > > > > > > > Signed-off-by: Julien Grall > > > > > > --- > > > > > >xen/arch/arm/guestcopy.c | 10 ++ > > > > > >xen/include/asm-arm/guest_access.h | 6 ++ > > > > > >2 files changed, 16 insertions(+) > > > > > > > > > > > > diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c > > > > > > index be53bee559..7958663970 100644 > > > > > > --- a/xen/arch/arm/guestcopy.c > > > > > > +++ b/xen/arch/arm/guestcopy.c > > > > > > @@ -110,6 +110,16 @@ unsigned long raw_copy_from_guest(void *to, > > > > > > const > > > > > > void __user *from, unsigned le > > > > > > COPY_from_guest | COPY_linear); > > > > > >} > > > > > >+unsigned long copy_to_guest_phys_flush_dcache(struct domain *d, > > > > > > + paddr_t gpa, > > > > > > + void *buf, > > > > > > + unsigned int len) > > > > > > +{ > > > > > > +/* P2M is shared between all vCPUs, so the vCPU used does not > > > > > > matter. > > > > > > */ > > > > > > > > > > Be very careful with this line of thinking. It is only works after > > > > > DOMCTL_max_vcpus has succeeded, and before that point, it is a latent > > > > > NULL pointer dereference. > > > > > > > > I really don't expect that function been used before DOMCT_max_vcpus is > > > > set. > > > > It is only used for hardware emulation or Xen loading image into the > > > > hardware > > > > domain memory. I could add a check d->vcpus to be safe. > > > > > > > > > > > > > > Also, what about vcpus configured with alternative views? > > > > > > > > It is not important because the underlying call is get_page_from_gfn > > > > that does > > > > not care about the alternative view (that function take a domain in > > > > parameter). I can update the comment. > > > Since this is a new function, would it make sense to take a struct > > > vcpu* as parameter, instead of a struct domain* ? > > > > Well, I suggested this patch this way because likely everyone will use with > > d->vcpus[0]. And then you would have to wonder why d->vcpus[0] and not > > d->vcpus[1]... > > Thinking a bit more to this, it might be better/safer to pass either a domain > or a vCPU to copy_guest. I can see 2 solutions: > 1# Introduce a union that use the same parameter: > union > { > struct > { > struct domain *d; > } ipa; > struct > { > struct vcpu *v; > } gva; > } > The structure here would be to ensure that it is clear that only > domain (resp. vcpu) should be used with ipa (resp. gva). > > 2# Have 2 parameters, vcpu and domain. > > Any opinions? I think that would be clearer. You could also add a paddr_t/vaddr_t correspondingly inside the union maybe. Well, you will have nameclash happening I think. And vaddr_t and paddr_t are confusing because you don't know which address you speak about (guest vs hypervisor). At least ipa/gpa, gva are known naming. Cheers, ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-next 07/16] xen/arm: Introduce copy_to_guest_phys_flush_dcache
On Fri, 8 Dec 2017, Julien Grall wrote: > On 8 Dec 2017 22:26, "Stefano Stabellini" wrote: > On Fri, 8 Dec 2017, Julien Grall wrote: > > On 06/12/17 12:27, Julien Grall wrote: > > > On 12/06/2017 01:26 AM, Stefano Stabellini wrote: > > > > On Thu, 23 Nov 2017, Julien Grall wrote: > > > > > Hi Andrew, > > > > > > > > > > On 23/11/17 18:49, Andrew Cooper wrote: > > > > > > On 23/11/17 18:32, Julien Grall wrote: > > > > > > > This new function will be used in a follow-up patch to copy > data to > > > > > > > the > > > > > > > guest > > > > > > > using the IPA (aka guest physical address) and then clean > the cache. > > > > > > > > > > > > > > Signed-off-by: Julien Grall > > > > > > > --- > > > > > > > xen/arch/arm/guestcopy.c | 10 ++ > > > > > > > xen/include/asm-arm/guest_access.h | 6 ++ > > > > > > > 2 files changed, 16 insertions(+) > > > > > > > > > > > > > > diff --git a/xen/arch/arm/guestcopy.c > b/xen/arch/arm/guestcopy.c > > > > > > > index be53bee559..7958663970 100644 > > > > > > > --- a/xen/arch/arm/guestcopy.c > > > > > > > +++ b/xen/arch/arm/guestcopy.c > > > > > > > @@ -110,6 +110,16 @@ unsigned long raw_copy_from_guest(void > *to, > > > > > > > const > > > > > > > void __user *from, unsigned le > > > > > > > COPY_from_guest | COPY_linear); > > > > > > > } > > > > > > > +unsigned long copy_to_guest_phys_flush_dcache(struct > domain *d, > > > > > > > + paddr_t gpa, > > > > > > > + void *buf, > > > > > > > + unsigned int > len) > > > > > > > +{ > > > > > > > + /* P2M is shared between all vCPUs, so the vCPU used > does not > > > > > > > matter. > > > > > > > */ > > > > > > > > > > > > Be very careful with this line of thinking. It is only works > after > > > > > > DOMCTL_max_vcpus has succeeded, and before that point, it is > a latent > > > > > > NULL pointer dereference. > > > > > > > > > > I really don't expect that function been used before > DOMCT_max_vcpus is > > > > > set. > > > > > It is only used for hardware emulation or Xen loading image > into the > > > > > hardware > > > > > domain memory. I could add a check d->vcpus to be safe. > > > > > > > > > > > > > > > > > Also, what about vcpus configured with alternative views? > > > > > > > > > > It is not important because the underlying call is > get_page_from_gfn > > > > > that does > > > > > not care about the alternative view (that function take a > domain in > > > > > parameter). I can update the comment. > > > > Since this is a new function, would it make sense to take a struct > > > > vcpu* as parameter, instead of a struct domain* ? > > > > > > Well, I suggested this patch this way because likely everyone will > use with > > > d->vcpus[0]. And then you would have to wonder why d->vcpus[0] and > not > > > d->vcpus[1]... > > > > Thinking a bit more to this, it might be better/safer to pass either > a domain > > or a vCPU to copy_guest. I can see 2 solutions: > > 1# Introduce a union that use the same parameter: > > union > > { > > struct > > { > > struct domain *d; > > } ipa; > > struct > > { > > struct vcpu *v; > > } gva; > > } > > The structure here would be to ensure that it is clear that > only > > domain (resp. vcpu) should be used with ipa (resp. gva). > > > > 2# Have 2 parameters, vcpu and domain. > > > > Any opinions? > > I think that would be clearer. You could also add a paddr_t/vaddr_t > correspondingly inside the union maybe. > > > Well, you will have nameclash happening I think. > > > And vaddr_t and paddr_t are confusing because you don't know which address > you speak about (guest vs hypervisor). At least ipa/gpa, gva are known naming. That's not what I meant. ipa and gva are good names. I was suggesting to put an additional address field inside the union to avoid the issue with paddr_t and vaddr_t discussed elsewhere (alpine.DEB.2.10.1712081313070.8052@sstabellini-ThinkPad-X260). I am happy either way, it was just a suggestion.___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listi
Re: [Xen-devel] [Qemu-devel] [PATCH] xen/pt: Set is_express to avoid out-of-bounds write
On Sat, 28 Oct 2017, Simon Gaiser wrote: > The passed-through device might be an express device. In this case the > old code allocated a too small emulated config space in > pci_config_alloc() since pci_config_size() returned the size for a > non-express device. This leads to an out-of-bound write in > xen_pt_config_reg_init(), which sometimes results in crashes. So set > is_express as already done for KVM in vfio-pci. > > Shortened ASan report: > > ==17512==ERROR: AddressSanitizer: heap-buffer-overflow on address > 0x61141648 at pc 0x55e0fdac51ff bp 0x7ffe4af07410 sp 0x7ffe4af07408 > WRITE of size 2 at 0x61141648 thread T0 > #0 0x55e0fdac51fe in memcpy > /usr/include/x86_64-linux-gnu/bits/string3.h:53 > #1 0x55e0fdac51fe in stw_he_p include/qemu/bswap.h:330 > #2 0x55e0fdac51fe in stw_le_p include/qemu/bswap.h:379 > #3 0x55e0fdac51fe in pci_set_word include/hw/pci/pci.h:490 > #4 0x55e0fdac51fe in xen_pt_config_reg_init > hw/xen/xen_pt_config_init.c:1991 > #5 0x55e0fdac51fe in xen_pt_config_init hw/xen/xen_pt_config_init.c:2067 > #6 0x55e0fdabcf4d in xen_pt_realize hw/xen/xen_pt.c:830 > #7 0x55e0fdf59666 in pci_qdev_realize hw/pci/pci.c:2034 > #8 0x55e0fdda7d3d in device_set_realized hw/core/qdev.c:914 > [...] > > 0x61141648 is located 8 bytes to the right of 256-byte region > [0x61141540,0x61141640) > allocated by thread T0 here: > #0 0x7ff596a94bb8 in __interceptor_calloc > (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xd9bb8) > #1 0x7ff57da66580 in g_malloc0 > (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x50580) > #2 0x55e0fdda7d3d in device_set_realized hw/core/qdev.c:914 > [...] > > Signed-off-by: Simon Gaiser Acked-by: Stefano Stabellini > --- > > I found this by debugging crashes and I'm not familiar with this code, > so I'm not sure if this has no unintended side effects. > > hw/xen/xen_pt.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c > index b6d71bb52a..90ffd45e7d 100644 > --- a/hw/xen/xen_pt.c > +++ b/hw/xen/xen_pt.c > @@ -946,6 +946,7 @@ static void xen_pci_passthrough_class_init(ObjectClass > *klass, void *data) > k->exit = xen_pt_unregister_device; > k->config_read = xen_pt_pci_read_config; > k->config_write = xen_pt_pci_write_config; > +k->is_express = 1; /* We might be */ > set_bit(DEVICE_CATEGORY_MISC, dc->categories); > dc->desc = "Assign an host PCI device with Xen"; > dc->props = xen_pci_passthrough_properties; > -- > 2.15.0.rc1 > > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 117015: trouble: broken/pass
flight 117015 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/117015/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: test-arm64-arm64-xl-xsm broken Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen 70f7b6ca0e8208034bdc91d20b2f311bbe63a0a9 baseline version: xen 43550972395f9a3a48bb4086a0faf0f8d442e37d Last test of basis 116956 2017-12-07 23:02:27 Z1 days Testing same since 117015 2017-12-08 22:03:21 Z0 days1 attempts People who touched revisions under test: Andre Przywara Andre Przywara Julien Grall Julien Grall Stefano Stabellini jobs: build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm broken test-amd64-amd64-xl-qemuu-debianhvm-i386 pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary broken-job test-arm64-arm64-xl-xsm broken Not pushing. commit 70f7b6ca0e8208034bdc91d20b2f311bbe63a0a9 Author: Andre Przywara Date: Thu Dec 7 16:14:08 2017 + ARM: VGIC: move gic_remove_irq_from_queues() gic_remove_irq_from_queues() was not only misnamed, it also has the wrong abstraction, as it should not live in gic.c. Move it into vgic.c and vgic.h, where it belongs to, and rename it on the way. Signed-off-by: Andre Przywara Reviewed-by: Stefano Stabellini commit 9630c5ae363b4cbf8eb61366530f40c80680af4d Author: Julien Grall Date: Wed Dec 6 14:51:37 2017 + xen/arm: gic-v3: Bail out if gicv3_cpu_init fail When system registers are not enabled, all the access to them will trap in EL2. In Xen, system registers will be enabled by gicv3_cpu_init only on success. As the rest of the code (e.g gicv3_hyp_init) relies on system register, it is better to bail out directly. This will save time on debugging early boot issue on GICv3 platform. Signed-off-by: Julien Grall Reviewed-by: Stefano Stabellini commit ac2d8d402370f6f93f82871f3b34ddb9a9ccae05 Author: Julien Grall Date: Wed Nov 29 17:46:35 2017 + xen/arm: Surround HSR_SYSREG macro value with () The value of the macro HCR_SYSREG is not surrounded by (). This means the behavior may change depend on how it is used. Thanksfully recent GCC will issue a warning for that. Signed-off-by: Julien Grall Reviewed-by: Stefano Stabellini commit b819187a15ecea7fe00cffded1bf454b8a6d7dd2 Author: Andre Przywara Date: Thu Oct 19 13:48:37 2017 +0100 ARM: vGIC: fix nr_irq definition The global variable "nr_irqs" is used for x86 and some common Xen code. To make the latter work easily for ARM, it was #defined to NR_IRQS. This not only violated the common habit of capitalizing macros, but also caused issues if one wanted to use a rather innocent "nr_irqs" as a local variable name or as a function parameter. Drop the optimization and make nr_irqs a normal variable for ARM also. Signed-off-by: Andre Przywara commit 2e9b1c655f060b5c4e68bc8499f02253babe1bbc Author: Andre Przywara Date: Thu Oct 19 13:48:36 2017 +0100 ARM: remove unneeded gic.h inclusions gic.h is supposed to hold defines and prototypes for the hardware side of the GIC interrupt controller. A lot of parts in Xen should not be bothered with that, as they either only care about the VGIC or use more generic interfaces. Remove unneeded inclusions of gic.h from files where they are actually not needed. Signed-off-by: Andre Przywara
[Xen-devel] [xen-4.10-testing test] 116961: regressions - FAIL
flight 116961 xen-4.10-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/116961/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-armhf-armhf-xl-credit2 16 guest-start/debian.repeat fail REGR. vs. 116762 Tests which are failing intermittently (not blocking): test-armhf-armhf-libvirt 7 xen-boot fail in 116940 pass in 116961 test-armhf-armhf-xl-xsm 6 xen-installfail pass in 116940 Tests which did not succeed, but are not blocking: test-amd64-i386-libvirt-qcow2 12 migrate-support-check fail in 116940 never pass test-armhf-armhf-xl-xsm 13 migrate-support-check fail in 116940 never pass test-armhf-armhf-xl-xsm 14 saverestore-support-check fail in 116940 never pass test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-xl-pvhv2-amd 12 guest-start fail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass version targeted for testing: xen aec99a1e0bc807ec0becb96728417fbfbd2e4139 baseline version: xen fd07c6d0f004286c7005e8d8f6fce26140da3746 L
[Xen-devel] [xen-unstable baseline-only test] 72527: regressions - FAIL
This run is configured for baseline tests only. flight 72527 xen-unstable real [real] http://osstest.xs.citrite.net/~osstest/testlogs/logs/72527/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-armhf-xsm 6 xen-build fail REGR. vs. 72522 build-armhf 6 xen-build fail REGR. vs. 72522 build-armhf-pvops 6 kernel-build fail REGR. vs. 72522 Tests which did not succeed, but are not blocking: test-armhf-armhf-xl-multivcpu 1 build-check(1) blocked n/a test-armhf-armhf-xl-midway1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a build-armhf-libvirt 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-armhf-armhf-examine 1 build-check(1) blocked n/a test-armhf-armhf-xl 1 build-check(1) blocked n/a test-armhf-armhf-xl-vhd 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit2 1 build-check(1) blocked n/a test-armhf-armhf-xl-rtds 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-xsm 1 build-check(1) blocked n/a test-armhf-armhf-xl-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail blocked in 72522 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stopfail blocked in 72522 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail like 72522 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail like 72522 test-amd64-amd64-qemuu-nested-intel 17 debian-hvm-install/l1/l2 fail like 72522 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 72522 test-amd64-amd64-xl-qemut-win10-i386 16 guest-localmigrate/x10 fail like 72522 test-amd64-amd64-examine 4 memdisk-try-append fail never pass test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-xl-pvhv2-amd 12 guest-start fail never pass test-amd64-i386-xl-qemut-ws16-amd64 10 windows-install fail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-qcow2 12 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-amd64-i386-xl-qemut-win10-i386 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass version targeted for testing: xen a04458bbf99f8fa64d727342938735727685f093 baseline version: xen 289adc1c56562d88e50b04245cd2027df8813bf4 Last test of basis72522 2017-12-06 22:19:47 Z2 days Testing same since72527 2017-12-08 17:45:32 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Jan Beulich jobs: build-amd64-xsm pass build-armhf-xsm fail build-i386-xsm pass build-amd64-xtf pass build-amd64 pass build-armhf fail build-i386 pass build-amd64-libvirt pass build-armhf-libvirt blocked build-i386-libvirt pass build-amd64-prev pass build-i386-prev pass build-amd64-pvopspass build-armhf-pvopsfail build-i386-pvops pass build-a
[Xen-devel] [xen-unstable-smoke test] 117017: trouble: broken/pass
flight 117017 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/117017/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: test-arm64-arm64-xl-xsm broken Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen 70f7b6ca0e8208034bdc91d20b2f311bbe63a0a9 baseline version: xen 43550972395f9a3a48bb4086a0faf0f8d442e37d Last test of basis 116956 2017-12-07 23:02:27 Z1 days Testing same since 117015 2017-12-08 22:03:21 Z0 days2 attempts People who touched revisions under test: Andre Przywara Andre Przywara Julien Grall Julien Grall Stefano Stabellini jobs: build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm broken test-amd64-amd64-xl-qemuu-debianhvm-i386 pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary broken-job test-arm64-arm64-xl-xsm broken Not pushing. commit 70f7b6ca0e8208034bdc91d20b2f311bbe63a0a9 Author: Andre Przywara Date: Thu Dec 7 16:14:08 2017 + ARM: VGIC: move gic_remove_irq_from_queues() gic_remove_irq_from_queues() was not only misnamed, it also has the wrong abstraction, as it should not live in gic.c. Move it into vgic.c and vgic.h, where it belongs to, and rename it on the way. Signed-off-by: Andre Przywara Reviewed-by: Stefano Stabellini commit 9630c5ae363b4cbf8eb61366530f40c80680af4d Author: Julien Grall Date: Wed Dec 6 14:51:37 2017 + xen/arm: gic-v3: Bail out if gicv3_cpu_init fail When system registers are not enabled, all the access to them will trap in EL2. In Xen, system registers will be enabled by gicv3_cpu_init only on success. As the rest of the code (e.g gicv3_hyp_init) relies on system register, it is better to bail out directly. This will save time on debugging early boot issue on GICv3 platform. Signed-off-by: Julien Grall Reviewed-by: Stefano Stabellini commit ac2d8d402370f6f93f82871f3b34ddb9a9ccae05 Author: Julien Grall Date: Wed Nov 29 17:46:35 2017 + xen/arm: Surround HSR_SYSREG macro value with () The value of the macro HCR_SYSREG is not surrounded by (). This means the behavior may change depend on how it is used. Thanksfully recent GCC will issue a warning for that. Signed-off-by: Julien Grall Reviewed-by: Stefano Stabellini commit b819187a15ecea7fe00cffded1bf454b8a6d7dd2 Author: Andre Przywara Date: Thu Oct 19 13:48:37 2017 +0100 ARM: vGIC: fix nr_irq definition The global variable "nr_irqs" is used for x86 and some common Xen code. To make the latter work easily for ARM, it was #defined to NR_IRQS. This not only violated the common habit of capitalizing macros, but also caused issues if one wanted to use a rather innocent "nr_irqs" as a local variable name or as a function parameter. Drop the optimization and make nr_irqs a normal variable for ARM also. Signed-off-by: Andre Przywara commit 2e9b1c655f060b5c4e68bc8499f02253babe1bbc Author: Andre Przywara Date: Thu Oct 19 13:48:36 2017 +0100 ARM: remove unneeded gic.h inclusions gic.h is supposed to hold defines and prototypes for the hardware side of the GIC interrupt controller. A lot of parts in Xen should not be bothered with that, as they either only care about the VGIC or use more generic interfaces. Remove unneeded inclusions of gic.h from files where they are actually not needed. Signed-off-by: Andre Przywara
[Xen-devel] [linux-arm-xen test] 116992: tolerable all pass - PUSHED
flight 116992 linux-arm-xen real [real] http://logs.test-lab.xenproject.org/osstest/logs/116992/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail like 107552 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 107552 test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-rtds 13 migrate-support-checkfail never pass test-arm64-arm64-xl-multivcpu 13 migrate-support-checkfail never pass test-arm64-arm64-xl-rtds 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-multivcpu 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-qcow2 12 migrate-support-checkfail never pass test-arm64-arm64-libvirt-qcow2 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-xsm 13 migrate-support-checkfail never pass test-armhf-armhf-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass version targeted for testing: linuxf829c1350f1b61684b919704970e84536971f62d baseline version: linux92ed32019d0dd22b796608079023ce42aa8a5a57 Last test of basis 107552 2017-04-19 23:49:01 Z 233 days Testing same since 116992 2017-12-08 15:36:32 Z0 days1 attempts 1557 people touched revisions under test, not listing them all jobs: build-arm64-xsm pass build-armhf-xsm pass build-arm64 pass build-armhf pass build-arm64-libvirt pass build-armhf-libvirt pass build-arm64-pvopspass build-armhf-pvopspass test-arm64-arm64-xl pass test-armhf-armhf-xl pass test-arm64-arm64-libvirt-xsm pass test-armhf-armhf-libvirt-xsm pass test-arm64-arm64-xl-xsm pass test-armhf-armhf-xl-xsm pass test-armhf-armhf-xl-arndale pass test-arm64-arm64-xl-credit2 pass test-armhf-armhf-xl-credit2 pass test-armhf-armhf-xl-cubietruck pass test-arm64-arm64-examine
[Xen-devel] [xen-unstable-smoke test] 117022: trouble: broken/pass
flight 117022 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/117022/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: test-arm64-arm64-xl-xsm broken Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen 70f7b6ca0e8208034bdc91d20b2f311bbe63a0a9 baseline version: xen 43550972395f9a3a48bb4086a0faf0f8d442e37d Last test of basis 116956 2017-12-07 23:02:27 Z1 days Testing same since 117015 2017-12-08 22:03:21 Z0 days3 attempts People who touched revisions under test: Andre Przywara Andre Przywara Julien Grall Julien Grall Stefano Stabellini jobs: build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm broken test-amd64-amd64-xl-qemuu-debianhvm-i386 pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary broken-job test-arm64-arm64-xl-xsm broken Not pushing. commit 70f7b6ca0e8208034bdc91d20b2f311bbe63a0a9 Author: Andre Przywara Date: Thu Dec 7 16:14:08 2017 + ARM: VGIC: move gic_remove_irq_from_queues() gic_remove_irq_from_queues() was not only misnamed, it also has the wrong abstraction, as it should not live in gic.c. Move it into vgic.c and vgic.h, where it belongs to, and rename it on the way. Signed-off-by: Andre Przywara Reviewed-by: Stefano Stabellini commit 9630c5ae363b4cbf8eb61366530f40c80680af4d Author: Julien Grall Date: Wed Dec 6 14:51:37 2017 + xen/arm: gic-v3: Bail out if gicv3_cpu_init fail When system registers are not enabled, all the access to them will trap in EL2. In Xen, system registers will be enabled by gicv3_cpu_init only on success. As the rest of the code (e.g gicv3_hyp_init) relies on system register, it is better to bail out directly. This will save time on debugging early boot issue on GICv3 platform. Signed-off-by: Julien Grall Reviewed-by: Stefano Stabellini commit ac2d8d402370f6f93f82871f3b34ddb9a9ccae05 Author: Julien Grall Date: Wed Nov 29 17:46:35 2017 + xen/arm: Surround HSR_SYSREG macro value with () The value of the macro HCR_SYSREG is not surrounded by (). This means the behavior may change depend on how it is used. Thanksfully recent GCC will issue a warning for that. Signed-off-by: Julien Grall Reviewed-by: Stefano Stabellini commit b819187a15ecea7fe00cffded1bf454b8a6d7dd2 Author: Andre Przywara Date: Thu Oct 19 13:48:37 2017 +0100 ARM: vGIC: fix nr_irq definition The global variable "nr_irqs" is used for x86 and some common Xen code. To make the latter work easily for ARM, it was #defined to NR_IRQS. This not only violated the common habit of capitalizing macros, but also caused issues if one wanted to use a rather innocent "nr_irqs" as a local variable name or as a function parameter. Drop the optimization and make nr_irqs a normal variable for ARM also. Signed-off-by: Andre Przywara commit 2e9b1c655f060b5c4e68bc8499f02253babe1bbc Author: Andre Przywara Date: Thu Oct 19 13:48:36 2017 +0100 ARM: remove unneeded gic.h inclusions gic.h is supposed to hold defines and prototypes for the hardware side of the GIC interrupt controller. A lot of parts in Xen should not be bothered with that, as they either only care about the VGIC or use more generic interfaces. Remove unneeded inclusions of gic.h from files where they are actually not needed. Signed-off-by: Andre Przywara