Re: [Xen-devel] [PATCH v3 32/35] OvmfPkg/PlatformBootManagerLib: Use a Xen console for ConOut/ConIn

[Laszlo Ersek]

On 07/22/19 19:06, Anthony PERARD wrote:
> On Wed, Jul 10, 2019 at 12:48:57PM +0200, Laszlo Ersek wrote:
>> On 07/04/19 16:42, Anthony PERARD wrote:
>>> On a Xen PVH guest, none of the existing serial or console interface
>>> works, so we add a new one, based on XenConsoleSerialPortLib, and
>>> implemented via SerialDxe.
>>>
>>> That is a simple console implementation that can works on both PVH
>>> guest and HVM guests, even if it rarely going to be use on HVM.
>>>
>>> Have PlatformBootManagerLib look for the new console, when running as a
>>> Xen guest.
>>>
>>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1689
>>> Signed-off-by: Anthony PERARD 
>>> ---
> 
>>> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c 
>>> b/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c
>>> index 36aab784d7..a9b1fe274a 100644
>>> --- a/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c
>>> +++ b/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c
>>> @@ -9,18 +9,19 @@
>>>  
>>>  #include "BdsPlatform.h"
>>>  #include 
>>> +#include 
>>>  
>>>  //
>>>  // Debug Agent UART Device Path structure
>>>  //
>>> -#pragma pack(1)
>>> +#pragma pack (1)
>>>  typedef struct {
>>>VENDOR_DEVICE_PATHVendorHardware;
>>>UART_DEVICE_PATH  Uart;
>>>VENDOR_DEVICE_PATHTerminalType;
>>>EFI_DEVICE_PATH_PROTOCOL  End;
>>>  } VENDOR_UART_DEVICE_PATH;
>>> -#pragma pack()
>>> +#pragma pack ()
>>>  
>>>  //
>>>  // USB Keyboard Device Path structure
>>> @@ -43,6 +44,18 @@ typedef struct {
>>>  } VENDOR_RAMFB_DEVICE_PATH;
>>>  #pragma pack ()
>>>  
>>> +//
>>> +// Xen Console Device Path structure
>>> +//
>>> +#pragma pack(1)
>>> +typedef struct {
>>> +  VENDOR_DEVICE_PATHVendorHardware;
>>> +  UART_DEVICE_PATH  Uart;
>>> +  VENDOR_DEVICE_PATHTerminalType;
>>> +  EFI_DEVICE_PATH_PROTOCOL  End;
>>> +} XEN_CONSOLE_DEVICE_PATH;
>>> +#pragma pack()
>>> +
>>
>> This version of the patch addresses all of my v2 review comments (either
>> by code changes or by explanations in the Notes section) -- thanks for that.
>>
>> However, when you arrived at my reuqest (6) in
>> ,
>> and searched the source file for "pack(" -- in order to insert a space
>> character before the opening paren --, the match was *not* around the
>> new XEN_CONSOLE_DEVICE_PATH structure. Instead, it was around the
>> preexistent VENDOR_UART_DEVICE_PATH structure. And so you fixed the
>> style for the old code, and not the new code.
>>
>> But: that's actually useful. Because now that I'm looking at
>> VENDOR_UART_DEVICE_PATH, it seems that we don't need the new type
>> XEN_CONSOLE_DEVICE_PATH at all. Is that right? So:
>>
>> (1) Please drop XEN_CONSOLE_DEVICE_PATH.
>>
>> (2) Please replace the comment
>>
>>   Debug Agent UART Device Path structure
>>
>> with
>>
>>   Vendor UART Device Path structure
>>
>> on VENDOR_UART_DEVICE_PATH.
>>
>> (3) Please preserve the "misplaced" whitespace fix, for "pack(", around
>> VENDOR_UART_DEVICE_PATH.
>>
>> (4) Please use VENDOR_UART_DEVICE_PATH as the type of gXenConsoleDevicePath.
>>
>> With those:
>>
>> Reviewed-by: Laszlo Ersek 
> 
> I'm going to add the following to the commit message:
> 
>   Since we use VENDOR_UART_DEVICE_PATH, fix its description and
>   coding style.
> 
> 

Thanks!
Laszlo

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

Re: [Xen-devel] Criteria / validation proposal: drop Xen

[Lars Kurth]

> On 12 Jul 2019, at 13:24, Peter Robinson  wrote:
> 
> On Fri, Jul 12, 2019 at 5:50 AM David Woodhouse  wrote:
>> 
>> 
>>> IIRC, what we have right now is a somewhat vague setup where we just
>>> have 'local', 'ec2' and 'openstack' columns. The instructions for
>>> "Amazon Web Services" just say "Launch an instance with the AMI under
>>> test". So we could probably stand to tighten that up a bit, and define
>>> specific instance type(s) that we want to test/block on.
>> 
>> I think we can define a set of instance types that would cover what it
>> makes sense to test. Do we still care about actual PV guests or only
>> HVM? I think it makes sense to test guests with Xen netback and blkback
>> rather than only ENA and NVMe, but Fedora probably wants to test the
>> latter two *anyway*.
>> 
>> Do we want to do this by making sure you have free credits to run the
>> appropriate tests directly... or is it better all round for us to just
>> do this on nightly builds for ourselves?
>> 
>> The latter brings me to a question that's been bugging me for a while —
>> how in $DEITY's name *do* I launch the latest official Fedora AMI
>> anyway? I can't find it through the normal GUI launch process and have
>> to go to getfedora.org and click around for a while because I find the
>> specific AMI ID for the that region, and then manually enter that to
>> launch the instance. Can't we fix that so I can just select 'Fedora 30'
>> with a single click? Whose heads do I have to bash together to make
>> that work?
> 
> So the easiest way to do this is by going to link [1] and select the
> cloud image "click to launch" it gives you a list of AWS regions and
> takes you direct to the AWS dialogs to run them.
> 
> [1] https://alt.fedoraproject.org/cloud/

David, Peter,
thanks for helping resolve this issue. It seems to me that testing against EC2 
Xen instances should indeed cover what most users need
Lars


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

Re: [Xen-devel] [PATCH v3 08/14] AMD/IOMMU: introduce 128-bit IRTE non-guest-APIC IRTE format

[Jan Beulich]

On 22.07.2019 17:43, Andrew Cooper wrote:
> On 22/07/2019 16:01, Jan Beulich wrote:
>> On 22.07.2019 15:36, Andrew Cooper wrote:
>>> On 22/07/2019 09:34, Jan Beulich wrote:
 On 19.07.2019 19:27, Andrew Cooper wrote:
> On 16/07/2019 17:38, Jan Beulich wrote:
>> @@ -142,7 +178,15 @@ static void free_intremap_entry(const st
>>  {
>>  union irte_ptr entry = get_intremap_entry(iommu, bdf, index);
>>  
>> -ACCESS_ONCE(entry.ptr32->raw[0]) = 0;
>> +if ( iommu->ctrl.ga_en )
>> +{
>> +ACCESS_ONCE(entry.ptr128->raw[0]) = 0;
>> +/* Low half (containing RemapEn) needs to be cleared first. */
>> +barrier();
> While this will function on x86, I still consider this buggy.  From a
> conceptual point of view, barrier() is not the correct construction to
> use, whereas smp_wmb() is.
 I think it's the 3rd time now that I respond saying that barrier() is
 as good or as bad as smp_wmb(), just for different reasons.
>>> barrier() and smp_wmb() are different constructs, with specific,
>>> *different* meanings.  From a programmers point of view, they should be
>>> considered black boxes of functionality.
>>>
>>> barrier() is for forcing the compiler to not reorder things.
>>>
>>> smp_wmb() is about the external visibility of writes, as observed by a
>>> different entity on a coherent fabric.
>> I'm afraid I disagree here: The "smp" in its name means "CPU", not
>> "entity" in your sentence.
> 
> Citation definitely needed.

Which I did provide in the earlier reply: If what you say was
intended to be that way, the !CONFIG_SMP definitions in Linux were
wrong, and ...

> The term SMP means Symmetric MultiProcessing, but no computer these days
> matches any of the traditional definitions.  You can thank the fact we
> are one of the fastest evolving industries in the world, and that the
> term you're using is more than 20 years old.

... would have been for a long time.

> In particular, it predates cache-coherent uncore devices.
> Cache-coherent devices by definition have the same ordering properties
> and constraints as cpus, because they are part of one shared (or dare I
> say, symmetric), cache-coherent domain.
> 
> How would your argument change if the IOMMU was a real CPU running real
> x86 code?  Its interface to the rest of the system would be identical,
> and in that case, it would obviously need an smp_{r,w}mb() pair for
> correctness reasons.  This is why smp_wmb() is the only appropriate
> construct to use.

It wouldn't change at all. What matters (as per above) is the
understanding the OS has, i.e. what is being surfaced to it as CPU.

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

Re: [Xen-devel] [PATCH v7] x86/emulate: Send vm_event from emulate

[Alexandru Stefan ISAILA]

On 19.07.2019 16:18, Jan Beulich wrote:
> On 19.07.2019 14:34, Alexandru Stefan ISAILA wrote:
>> On 18.07.2019 15:58, Jan Beulich wrote:
>>> On 03.07.2019 12:56, Alexandru Stefan ISAILA wrote:

 Currently, we are fully emulating the instruction at RIP when the hardware 
 sees
 an EPT fault with npfec.kind != npfec_kind_with_gla. This is, however,
 incorrect, because the instruction at RIP might legitimately cause an
 EPT fault of its own while accessing a _different_ page from the original 
 one,
 where A/D were set.
 The solution is to perform the whole emulation,
>>>
>>> Above you said fully emulating such an insn is incorrect. To me the
>>> two statements contradict one another.
>>>
 while ignoring EPT restrictions
 for the walk part, and taking them into account for the "actual" emulating 
 of
 the instruction at RIP.
>>>
>>> So the "ignore" part here is because the walk doesn't currently send
>>> any events? That's an omission after all, which ultimately wants to
>>> get fixed. This in turn makes me wonder whether there couldn't be
>>> cases where a monitor actually wants to see these violations, too.
>>> After all one may be able to abuse to page walker to set bits in
>>> places you actually care to protect from undue modification.
>>
>> There is no need for events from page walk. Further work will have to be
>> done, when page-walk will send events, so that we can toggle that new
>> feature on/off.
> 
> Please can you move over to thinking in more general terms,
> not just what you need for your application. In this case
> "There is no need" != "We don't have a need for". And I think
> the VM event _interface_ should be arranged in a way that it
> already accounts for eventually correct behavior of the page
> walk paths.
> 

I'm not sure how future code for sending events form page-walk will be 
but I will try to make this patch have some checks in place so that it 
will work the same.

 After the emulation stops we'll call hvm_vm_event_do_resume() again after 
 the
 introspection agent treats the event and resumes the guest. There, the
 instruction at RIP will be fully emulated (with the EPT ignored) if the
 introspection application allows it, and the guest will continue to run 
 past
 the instruction.

 We use hvmemul_map_linear_addr() to intercept r/w access and
 __hvm_copy() to intercept exec access.
>>>
>>> Btw I continue to be unhappy about this asymmetry. Furthermore in
>>> the former case you only handle write and rmw accesses, but not
>>> reads afaics. I assume you don't care about reads, but this should
>>> then be made explicit. Furthermore EPT allows read protection, and
>>> there are p2m_access_w, p2m_access_wx, and p2m_access_x, so I guess
>>> ignoring reads can at best be an option picked by the monitor, not
>>> something to be left out of the interface altogether.
>>
>> That is correct, we are not interested in read events but there is
>> another problem, we are checking access and pfec to fill the event flag
>> and pfec only has a write flag(PFEC_write_access), in __hvmemul_read()
>> pfec only gets PFEC_page_present and there is no way to differentiate
>> write from read.
> 
> By the PFEC model, anything that's not a write or insn fetch is a
> read. The main anomaly is elsewhere: The write flag is also going
> to be set for RMW operations.
> 
 hvm_emulate_send_vm_event() can return false if there was no violation,
 if there was an error from monitor_traps() or p2m_get_mem_access().
>>>
>>> As said before - I don't think errors and lack of a violation can
>>> sensibly be treated the same way. Is the implication perhaps that
>>> emulation then will fail later anyway? If so, is such an
>>> assumption taking into consideration possible races?
>>
>> The only place that I can see a problem is the error from
>> monitor_traps(). That can be checked and accompanied by a warning msg.
> 
> How would a warning message help?
> 
>> Or if you can give me a different idea to go forward with this issue I
>> will be glad to review it.
> 
> I'm afraid you'll have to first of all give me an idea what the
> correct action is in case of such an error. And once you've done
> so, I'm pretty sure you'll recognize yourself whether the current
> code you have is appropriate (and I'll then know whether I want
> to insist on you changing the code).
> 

So I think that the return of hvm_emulate_send_vm_event() should not use 
the return of monitor_traps(). By the time monitor_traps() is called we 
are sure that there is a violation and emulation should stop regardless 
if the event was sent or not. In this idea the last return should be true.

 --- a/xen/arch/x86/hvm/hvm.c
 +++ b/xen/arch/x86/hvm/hvm.c
 @@ -3224,6 +3224,14 @@ static enum hvm_translation_result __hvm_copy(
  return HVMTRANS_bad_gfn_to_mfn;
  }
  
 +if ( unlikely(v->arch.vm_event) &&

Re: [Xen-devel] [PATCH v3 08/14] AMD/IOMMU: introduce 128-bit IRTE non-guest-APIC IRTE format

[Jan Beulich]

On 23.07.2019 10:13, Jan Beulich wrote:
> On 22.07.2019 17:43, Andrew Cooper wrote:
>> How would your argument change if the IOMMU was a real CPU running real
>> x86 code?  Its interface to the rest of the system would be identical,
>> and in that case, it would obviously need an smp_{r,w}mb() pair for
>> correctness reasons.  This is why smp_wmb() is the only appropriate
>> construct to use.
> 
> It wouldn't change at all. What matters (as per above) is the
> understanding the OS has, i.e. what is being surfaced to it as CPU.

Oh, btw - I've got curious whether we could use Linux sources for
arbitration. What I found though is that they don't use any barrier
at all - see modify_irte_ga().

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

Re: [Xen-devel] [PATCH] vunmap: let vunmap align virtual address by itself

[Andrii Anisov]

Julien, Jan, Andrew,

The problem addressed by [1] causes random ARM64 boot fails dependent on 
hypervisor code changes.
Yet more generic solution was requested by Andrew and supported by Julien [2].

How to proceed with this particular patch?
As I understand, Jan doubts we should move page alignment to vunmap(), while 
Julien and Andrew wanted the commit message clarification.
Can we have an agreement on approach here?

[1] https://lists.xenproject.org/archives/html/xen-devel/2019-07/msg01167.html
[2] https://lists.xenproject.org/archives/html/xen-devel/2019-07/msg01129.html

--
Sincerely,
Andrii Anisov.

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

Re: [Xen-devel] Xen Hypervisor porting on Raspberry Pi 3B+/4

[Sushant Bhangale]

Hi,

Awaited for your input.

Regards,
Sushant

From: Sushant Bhangale
Sent: Friday, July 12, 2019 7:52 PM
To: xen-devel@lists.xenproject.org
Cc: Pranav Paralikar ; Nikhil Wadke 
; sstabell...@kernel.org; julien.gr...@arm.com; 
lars.ku...@citrix.com
Subject: Xen Hypervisor porting on Raspberry Pi 3B+/4

Hi,

We are planning to port Xen Hypervisor on Raspberry PI 3B+(A53 Processor) OR 
Raspberry PI 4 (A72 Processor). For this purpose we find out the below tools,

Tools
Versions
Path
Cross Compiler
gcc-linaro-arm-none-eabi-4.9-2014.09_linux
.
gcc-linaro-7.2.1-2017.11-x86_64_aarch64-linux-gnu
.
arm-linux-gnueabi-gcc
.
Bootloader
U-Boot
https://github.com/u-boot/u-boot
Linux Kernel
v3.18-rc15
https://wiki.xenproject.org/wiki/Xen_ARM_with_Virtualization_Extensions
linux-4.2.tar.xz
https://www.kernel.org/pub/linux/kernel/v4.x/linux-4.2.tar.xz
Root Filesystem
BusyBox-1.30.1 or Older
https://busybox.net/
Xen Hypervisor Source code
Xen Hypervisor (v4.12 Stable version)
https://github.com/bkrepo/xen.git
Processor 1
Broadcom BCM5871X (A53)
Raspberry Pi 3B / 3B+
 Processor 2
Broadcom BCM2711 (A72)
Raspberry Pi 4

Request you to let us know whether above selected tools in the table is OK or 
NOT.

Also, if any document related to porting of Xen hypervisor on Raspberry PI 
3B+(A53 Processor) OR Raspberry PI 4 (A72 Processor) is available with you, 
please share the same.

Looking forward for your response.

Regards,
Sushant Bhangale
Senior Engineer - EDP
L&T TECHNOLOGY SERVICES LIMITED
8th Floor Building No,1,
Thane Belapur Road, Mindspace
Airoli, Navi Mumbai :India 400708


Tel: +91 22  6105 8289 | Mobile: +91 76 2033 0707
www.Ltts.com
[cid:image007.jpg@01D29F47.C0ED3070]


L&T Technology Services Ltd

www.LTTS.com

This Email may contain confidential or privileged information for the intended 
recipient (s). If you are not the intended recipient, please do not use or 
disseminate the information, notify the sender and delete it from your system.
--
To unsubscribe from this group and stop receiving emails from it, send an email 
to 
xenprojecthelp+unsubscr...@linuxfoundation.org.

L&T Technology Services Ltd

www.LTTS.com

This Email may contain confidential or privileged information for the intended 
recipient (s). If you are not the intended recipient, please do not use or 
disseminate the information, notify the sender and delete it from your system.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [linux-next test] 139249: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-arm64-arm64-libvirt-xsm 16 guest-start/debian.repeat fail REGR. vs. 139237

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-armhf-armhf-examine  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-vhd   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-arndale   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-credit1   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-cubietruck  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-credit2   1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-rtds  1 build-check(1)   blocked  n/a
 test-arm64-arm64-examine 11 examine-serial/bootloaderfail  like 139223
 test-amd64-i386-xl-qemuu-debianhvm-amd64  7 xen-boot  fail like 139237
 test-amd64-i386-xl-raw7 xen-boot fail  like 139237
 test-amd64-i386-examine   8 reboot   fail  like 139237
 test-amd64-i386-xl-xsm7 xen-boot fail  like 139237
 test-amd64-i386-xl7 xen-boot fail  like 139237
 test-amd64-i386-qemut-rhel6hvm-intel  7 xen-boot  fail like 139237
 test-amd64-i386-libvirt-pair 10 xen-boot/src_hostfail  like 139237
 test-amd64-i386-libvirt-pair 11 xen-boot/dst_hostfail  like 139237
 test-amd64-i386-xl-shadow 7 xen-boot fail  like 139237
 test-amd64-i386-xl-qemut-win10-i386  7 xen-boot   fail like 139237
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-boot fail like 
139237
 test-amd64-i386-qemuu-rhel6hvm-amd  7 xen-bootfail like 139237
 test-amd64-i386-xl-qemuu-win10-i386  7 xen-boot   fail like 139237
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm  7 xen-boot   fail like 139237
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-boot fail like 
139237
 test-amd64-i386-libvirt-xsm   7 xen-boot fail  like 139237
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow  7 xen-boot   fail like 139237
 test-amd64-i386-qemut-rhel6hvm-amd  7 xen-bootfail like 139237
 test-amd64-i386-pair 10 xen-boot/src_hostfail  like 139237
 test-amd64-i386-pair 11 xen-boot/dst_hostfail  like 139237
 test-amd64-i386-xl-qemut-ws16-amd64  7 xen-boot   fail like 139237
 test-amd64-i386-freebsd10-i386  7 xen-bootfail like 139237
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm  7 xen-boot fail like 139237
 test-amd64-i386-xl-qemuu-ws16-amd64  7 xen-boot   fail like 139237
 test-amd64-i386-xl-qemut-debianhvm-i386-xsm  7 xen-boot   fail like 139237
 test-amd64-i386-xl-qemut-debianhvm-amd64  7 xen-boot  fail like 139237
 test-amd64-i386-libvirt   7 xen-boot fail  like 139237
 test-amd64-i386-qemuu-rhel6hvm-intel  7 xen-boot  fail like 139237
 test-amd64-i386-xl-qemut-win7-amd64  7 xen-boot   fail like 139237
 test-amd64-i386-xl-pvshim 7 xen-boot fail  like 139237
 test-amd64-i386-xl-qemuu-win7-amd64  7 xen-boot   fail like 139237
 test-amd64-i386-freebsd10-amd64  7 xen-boot   fail like 139237
 test-amd64-i386-xl-qemuu-ovmf-amd64  7 xen-boot   fail like 139237
 build-armhf-pvops 6 kernel-build fail  like 139237
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 139237
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 139237
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 139237
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 139237
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-check  

Re: [Xen-devel] [PATCH v3 09/35] OvmfPkg/OvmfXen: use a TimerLib instance that depends only on the CPU

[Roger Pau Monné]

On Mon, Jul 22, 2019 at 09:28:20PM +0200, Laszlo Ersek wrote:
> On 07/22/19 15:49, Anthony PERARD wrote:
> > On Mon, Jul 15, 2019 at 04:22:19PM +0200, Roger Pau Monné wrote:
> >> On Thu, Jul 04, 2019 at 03:42:07PM +0100, Anthony PERARD wrote:
> >>> ACPI Timer does not work in a PVH guest, but local APIC works on both
> >>
> >> This is not accurate. It's not that the ACPI timer doesn't work, it's
> >> just that it's not present. The PM_TMR_BLK should be set to 0 to
> >> indicate the lack of PM timer support, or else there's something
> >> broken.
> > 
> > I'll reword that first sentence.
> > 
> > OVMF doesn't look at the PM_TMR_BLK value when initializing that timer,
> > it only looks at the PCI host bridge device ID because OVMF is built
> > with QEMU in mind and there are only two possibles choices, QEMU is
> > running with a piix or q35 machine type, I think.
> 
> We should split this statement in two. :)
> 
> OVMF doesn't look at ACPI payload because it is a design goal to keep
> the guest firmware un-enlightened about such ACPI contents that arrive
> from the hypervisor. Parsing ACPI in firmware always looks attractive
> until someone actually writes the code, and then it always ends in
> misery -- at the latest when people realize they have to parse AML.
> Parsing ACPI is only feasible when you have a full-blown ACPICA (or
> similar) subsystem, and edk2 doesn't. Therefore, OVMF looks at either
> hardware, or specialized paravirt information channels such as fw_cfg
> files, that are easy to parse by design.

IMO passing information using such side-channels always looks
attractive at first sight, until you realize at some point later that
you just have ended up with a completely custom interface that
duplicates ACPI. Having that said, Xen manages to get most of what it
needs from static ACPI tables, but I'm not sure if OVMF could manage
to do so also.

Xen has quite a lot of baggage here, like using xenstore/xenbus
instead of PCI, or custom 'start info pages' instead of ACPI, which we
are currently trying to partially move away from when possible.

Thanks, Roger.

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

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm  7 xen-boot  fail REGR. vs. 139230

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 139230
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 139230
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 139230
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 139230
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 139230
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-amd64-libvirt-xsm 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-arm64-arm64-xl-seattle  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  14 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail 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-xl-credit1  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 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-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   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 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  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-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 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-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass

version targeted for testing:
 qemuu4a10982c320740d2d0565180d901a69b043dc282
baseline version:
 qemuue2b47666fe1544959c89bd3ed159e9e37cc9fc73

Last test of basis   139230  2019-07-21 05:33:47 Z2 days
Testing same since   139251  2019-07-22 09:36:11 Z0 days1 attempts


People who touched revisions under test:
  Andrey Shinkevich 
  Kevin Wolf 
  Max Reitz 
  Peter Maydell 

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  pass
 build-i386-xsm   pass
 build-amd64   

Re: [Xen-devel] [PATCH] vunmap: let vunmap align virtual address by itself

[Jan Beulich]

On 23.07.2019 10:48, Andrii Anisov wrote:
> Julien, Jan, Andrew,
> 
> The problem addressed by [1] causes random ARM64 boot fails dependent on 
> hypervisor code changes.
> Yet more generic solution was requested by Andrew and supported by Julien [2].
> 
> How to proceed with this particular patch?
> As I understand, Jan doubts we should move page alignment to vunmap(), while 
> Julien and Andrew wanted the commit message clarification.
> Can we have an agreement on approach here?
> 
> [1] https://lists.xenproject.org/archives/html/xen-devel/2019-07/msg01167.html
> [2] https://lists.xenproject.org/archives/html/xen-devel/2019-07/msg01129.html

First of all, let me quote Linux'es code:

static void __vunmap(const void *addr, int deallocate_pages)
{
struct vm_struct *area;

if (!addr)
return;

if (WARN(!PAGE_ALIGNED(addr), "Trying to vfree() bad address (%p)\n",
addr))
return;

As long as we aim to have a reasonable level of compatibility of
similar interfaces, we should not go the suggested route.

Beyond that I continue to be of the opinion that it should be
all-or-nothing: Any pointer pointing anywhere at or inside the
region should be accepted, or just the one pointing precisely at
the start.

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

[Xen-devel] [PATCH 1/2] xen/sched: fix locking in restore_vcpu_affinity()

[Juergen Gross]

Commit 0763cd2687897b55e7 ("xen/sched: don't disable scheduler on cpus
during suspend") removed a lock in restore_vcpu_affinity() which needs
to stay: cpumask_scratch_cpu() must be protected by the scheduler
lock. restore_vcpu_affinity() is being called by thaw_domains(), so
with multiple domains in the system another domain might already be
running and the scheduler might make use of cpumask_scratch_cpu()
already.

Signed-off-by: Juergen Gross 
---
 xen/common/schedule.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index 25f6ab388d..89bc259ae4 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -708,6 +708,8 @@ void restore_vcpu_affinity(struct domain *d)
  * set v->processor of each of their vCPUs to something that will
  * make sense for the scheduler of the cpupool in which they are in.
  */
+lock = vcpu_schedule_lock_irq(v);
+
 cpumask_and(cpumask_scratch_cpu(cpu), v->cpu_hard_affinity,
 cpupool_domain_cpumask(d));
 if ( cpumask_empty(cpumask_scratch_cpu(cpu)) )
@@ -731,6 +733,9 @@ void restore_vcpu_affinity(struct domain *d)
 
 v->processor = cpumask_any(cpumask_scratch_cpu(cpu));
 
+spin_unlock_irq(lock);
+
+   /* v->processor might have changed, so reacquire the lock. */
 lock = vcpu_schedule_lock_irq(v);
 v->processor = sched_pick_cpu(vcpu_scheduler(v), v);
 spin_unlock_irq(lock);
-- 
2.16.4


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

[Xen-devel] [PATCH 2/2] xen: merge temporary vcpu pinning scenarios

[Juergen Gross]

Today there are three scenarios which are pinning vcpus temporarily to
a single physical cpu:

- NMI/MCE injection into PV domains
- wait_event() handling
- vcpu_pin_override() handling

Each of those cases are handled independently today using their own
temporary cpumask to save the old affinity settings.

The three cases can be combined as the two latter cases will only pin
a vcpu to the physical cpu it is already running on, while
vcpu_pin_override() is allowed to fail.

So merge the three temporary pinning scenarios by only using one
cpumask and a per-vcpu bitmask for specifying which of the three
scenarios is currently active (they are allowed to nest).

Note that we don't need to call domain_update_node_affinity() as we
are only pinning for a brief period of time.

Signed-off-by: Juergen Gross 
---
 xen/arch/x86/pv/traps.c | 20 +---
 xen/arch/x86/traps.c|  8 ++--
 xen/common/domain.c |  4 +---
 xen/common/schedule.c   | 35 +++
 xen/common/wait.c   | 26 --
 xen/include/xen/sched.h |  8 +---
 6 files changed, 40 insertions(+), 61 deletions(-)

diff --git a/xen/arch/x86/pv/traps.c b/xen/arch/x86/pv/traps.c
index 1740784ff2..37dac300ba 100644
--- a/xen/arch/x86/pv/traps.c
+++ b/xen/arch/x86/pv/traps.c
@@ -151,25 +151,7 @@ static void nmi_mce_softirq(void)
 
 BUG_ON(st->vcpu == NULL);
 
-/*
- * Set the tmp value unconditionally, so that the check in the iret
- * hypercall works.
- */
-cpumask_copy(st->vcpu->cpu_hard_affinity_tmp,
- st->vcpu->cpu_hard_affinity);
-
-if ( (cpu != st->processor) ||
- (st->processor != st->vcpu->processor) )
-{
-
-/*
- * We are on a different physical cpu.  Make sure to wakeup the vcpu on
- * the specified processor.
- */
-vcpu_set_hard_affinity(st->vcpu, cpumask_of(st->processor));
-
-/* Affinity is restored in the iret hypercall. */
-}
+vcpu_set_tmp_affinity(st->vcpu, st->processor, VCPU_AFFINITY_NMI);
 
 /*
  * Only used to defer wakeup of domain/vcpu to a safe (non-NMI/MCE)
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 25b4b47e5e..22bb790858 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1601,12 +1601,8 @@ void async_exception_cleanup(struct vcpu *curr)
 return;
 
 /* Restore affinity.  */
-if ( !cpumask_empty(curr->cpu_hard_affinity_tmp) &&
- !cpumask_equal(curr->cpu_hard_affinity_tmp, curr->cpu_hard_affinity) )
-{
-vcpu_set_hard_affinity(curr, curr->cpu_hard_affinity_tmp);
-cpumask_clear(curr->cpu_hard_affinity_tmp);
-}
+if ( curr->affinity_broken & VCPU_AFFINITY_NMI )
+vcpu_set_tmp_affinity(curr, -1, VCPU_AFFINITY_NMI);
 
 if ( !(curr->async_exception_mask & (curr->async_exception_mask - 1)) )
 trap = __scanbit(curr->async_exception_mask, VCPU_TRAP_NONE);
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 55aa759b75..e8e850796e 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -133,7 +133,6 @@ static void vcpu_info_reset(struct vcpu *v)
 static void vcpu_destroy(struct vcpu *v)
 {
 free_cpumask_var(v->cpu_hard_affinity);
-free_cpumask_var(v->cpu_hard_affinity_tmp);
 free_cpumask_var(v->cpu_hard_affinity_saved);
 free_cpumask_var(v->cpu_soft_affinity);
 
@@ -161,7 +160,6 @@ struct vcpu *vcpu_create(
 grant_table_init_vcpu(v);
 
 if ( !zalloc_cpumask_var(&v->cpu_hard_affinity) ||
- !zalloc_cpumask_var(&v->cpu_hard_affinity_tmp) ||
  !zalloc_cpumask_var(&v->cpu_hard_affinity_saved) ||
  !zalloc_cpumask_var(&v->cpu_soft_affinity) )
 goto fail;
@@ -1269,7 +1267,7 @@ int vcpu_reset(struct vcpu *v)
 v->async_exception_mask = 0;
 memset(v->async_exception_state, 0, sizeof(v->async_exception_state));
 #endif
-cpumask_clear(v->cpu_hard_affinity_tmp);
+v->affinity_broken = 0;
 clear_bit(_VPF_blocked, &v->pause_flags);
 clear_bit(_VPF_in_reset, &v->pause_flags);
 
diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index 89bc259ae4..d4de74f9c8 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -1106,47 +1106,58 @@ void watchdog_domain_destroy(struct domain *d)
 kill_timer(&d->watchdog_timer[i]);
 }
 
-int vcpu_pin_override(struct vcpu *v, int cpu)
+int vcpu_set_tmp_affinity(struct vcpu *v, int cpu, uint8_t reason)
 {
 spinlock_t *lock;
 int ret = -EINVAL;
+bool migrate;
 
 lock = vcpu_schedule_lock_irq(v);
 
 if ( cpu < 0 )
 {
-if ( v->affinity_broken )
+if ( v->affinity_broken & reason )
 {
-sched_set_affinity(v, v->cpu_hard_affinity_saved, NULL);
-v->affinity_broken = 0;
 ret = 0;
+v->affinity_broken &= ~reason;
 }
+if ( !ret && !v->affinity_broken )
+sched_set_affinity(v, v->cpu_hard_affinity_saved, NULL);
 }
 

[Xen-devel] [PATCH 0/2] xen: fix/enhance temporary vcpu pinning

[Juergen Gross]

While trying to handle temporary vcpu pinnings in a sane way in my
core scheduling series I stumbled over a bug and found a nice way to
simplify the temporary pinning cases.

I'm sending the two patches independently from my core scheduling
series as they should be considered even without core scheduling.

Juergen Gross (2):
  xen/sched: fix locking in restore_vcpu_affinity()
  xen: merge temporary vcpu pinning scenarios

 xen/arch/x86/pv/traps.c | 20 +---
 xen/arch/x86/traps.c|  8 ++--
 xen/common/domain.c |  4 +---
 xen/common/schedule.c   | 40 
 xen/common/wait.c   | 26 --
 xen/include/xen/sched.h |  8 +---
 6 files changed, 45 insertions(+), 61 deletions(-)

-- 
2.16.4


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

Re: [Xen-devel] [PATCH v3 24/35] OvmfPkg/XenPlatformPei: Rework memory detection

[Roger Pau Monné]

On Mon, Jul 22, 2019 at 03:53:19PM +0100, Anthony PERARD wrote:
> On Mon, Jul 15, 2019 at 04:15:21PM +0200, Roger Pau Monné wrote:
> > On Thu, Jul 04, 2019 at 03:42:22PM +0100, Anthony PERARD wrote:
> > > When running as a Xen PVH guest, there is no CMOS to read the memory
> > > size from.  Rework GetSystemMemorySize(Below|Above)4gb() so they can
> > > works without CMOS by reading the e820 table.
> > > 
> > > Rework XenPublishRamRegions for PVH, handle the Reserve type and explain
> > > about the ACPI type. MTRR settings aren't modified anymore, on HVM, it's
> > > already done by hvmloader, on PVH it is supposed to have sane default.
> > > 
> > > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1689
> > > Signed-off-by: Anthony PERARD 
> > > Acked-by: Laszlo Ersek 
> > > ---
> > > 
> > > Notes:
> > > Comment for Xen people:
> > > About MTRR, should we redo the setting in OVMF? Even if in both case 
> > > of
> > > PVH and HVM, something would have setup the default type to write back
> > > and handle a few other ranges like PCI hole, hvmloader for HVM or and
> > > libxc I think for PVH.
> > 
> > That's a tricky question. Ideally we would like the firmware (OVMF) to
> > take care of that, because it already has code to do so. Problem here
> > is that PVH can also be booted without firmware, in which case it
> > needs the hypervisor to have setup some sane initial MTRR state.
> > 
> > The statement in the PVH document about initial MTRR state is vague
> > enough that allows Xen to boot into the guest with a minimal MTRR
> > state, that can for example not contain UC regions for the MMIO
> > regions of passed through devices, hence I think OVMF should be in
> > charge of creating a more complete MTRR state if possible.
> > 
> > Is this something OVMF already has logic for?
> 
> Well, there are some logic but it's for QEMU (and uses an interface that
> isn't available when running on Xen, fwcfg).
> 
> The logic that was there for Xen HVM was very simple, a single set
> cache-write-back for the RAM, that's why I remove it (and because I'm
> not sure yet I figured out how to run the mtrr functions correctly in
> OVMF).
> 
> I probably going to have to write a new logic which would rewrite the
> MTRR from scratch instead of relying on the existing setup.
> 
> > Also accounting for the MMIO regions of devices?
> 
> I'll have to dig deeper into OVMF codes, and PCI device handling. On
> HVM, we have a different logic than the one for QEMU, OVMF only scan
> what hvmloader have done instead of re-setup the pci devices. I'm
> probably missing other stuff.

MTRR setup it's always a PITA, I was hoping OVMF could manage to do
that based on the memory map plus scanning for devices and positioning
BARs, but if it gets the information from other side-channels it's
going to be more complicated.

Anyway, something to improve in the future in order to get rid of
hvmloader.

> > > diff --git a/OvmfPkg/XenPlatformPei/MemDetect.c 
> > > b/OvmfPkg/XenPlatformPei/MemDetect.c
> > > index cb7dd93ad6..3e33e7f414 100644
> > > --- a/OvmfPkg/XenPlatformPei/MemDetect.c
> > > +++ b/OvmfPkg/XenPlatformPei/MemDetect.c
> > > @@ -96,6 +96,47 @@ Q35TsegMbytesInitialization (
> > >mQ35TsegMbytes = ExtendedTsegMbytes;
> > >  }
> > >  
> > > +STATIC
> > > +UINT64
> > > +GetHighestSystemMemoryAddress (
> > > +  BOOLEAN   Below4gb
> > > +  )
> > > +{
> > > +  EFI_E820_ENTRY64*E820Map;
> > > +  UINT32  E820EntriesCount;
> > > +  EFI_E820_ENTRY64*Entry;
> > > +  EFI_STATUS  Status;
> > > +  UINT32  Loop;
> > > +  UINT64  HighestAddress;
> > > +  UINT64  EntryEnd;
> > > +
> > > +  HighestAddress = 0;
> > > +
> > > +  Status = XenGetE820Map (&E820Map, &E820EntriesCount);
> > 
> > You could maybe initialize this as a global to avoid having to issue
> > a hypercall each time you need to get something from the memory map.
> 
> That function does that, it only make the hypercall once. (The hypercall
> can only be made once anyway, the second time Xen doesn't return the
> map.)

Why? I'm looking at the implementation in Xen of XENMEM_memory_map and
I'm not sure I see how/why the hypercall can only be made once. AFAICT
you should be able to call XENMEM_memory_map multiple times without
issues, or else it's a bug somewhere.

> > > +  ASSERT_EFI_ERROR (Status);
> > > +
> > > +  for (Loop = 0; Loop < E820EntriesCount; Loop++) {
> > > +Entry = E820Map + Loop;
> > > +EntryEnd = Entry->BaseAddr + Entry->Length;
> > > +
> > > +if (Entry->Type == EfiAcpiAddressRangeMemory &&
> > > +EntryEnd > HighestAddress) {
> > > +
> > > +  if (Below4gb && (EntryEnd <= BASE_4GB)) {
> > > +HighestAddress = EntryEnd;
> > > +  } else if (!Below4gb && (EntryEnd >= BASE_4GB)) {
> > > +HighestAddress = EntryEnd;
> > > +  }
> > > +}
> > > +  }
> > > +
> > > +  //
> > > +  // Round down the end address.
> > > +  //
> > > +  HighestAddress &= ~(U

Re: [Xen-devel] [PATCH] vunmap: let vunmap align virtual address by itself

[Andrii Anisov]

Hello Jan,

On 23.07.19 12:12, Jan Beulich wrote:

Beyond that I continue to be of the opinion that it should be
all-or-nothing: Any pointer pointing anywhere at or inside the
region should be accepted, or just the one pointing precisely at
the start.


It's reasonable enough and I agree with that.
Sorry for missing this point.

--
Sincerely,
Andrii Anisov.

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

Re: [Xen-devel] [PATCH v4 3/6] pci: switch pci_conf_read32 to use pci_sbdf_t

[Jan Beulich]

On 19.07.2019 16:07, Roger Pau Monne wrote:
> This reduces the number of parameters of the function to two, and
> simplifies some of the calling sites.
> 
> While there convert {IGD/IOH}_DEV to be a pci_sbdf_t itself instead of
> a device number.
> 
> Signed-off-by: Roger Pau Monné 
> Acked-by: Brian Woods 
> Reviewed-by: Kevin Tian 

Acked-by: Jan Beulich 
with one further remark (easily addressed while committing):

> @@ -128,9 +128,9 @@ static void __init map_igd_reg(void)
>   if ( igd_reg_va )
>   return;
>   
> -igd_mmio   = pci_conf_read32(0, 0, IGD_DEV, 0, PCI_BASE_ADDRESS_1);
> +igd_mmio   = pci_conf_read32(IGD_DEV, PCI_BASE_ADDRESS_1);
>   igd_mmio <<= 32;
> -igd_mmio  += pci_conf_read32(0, 0, IGD_DEV, 0, PCI_BASE_ADDRESS_0);
> +igd_mmio  += pci_conf_read32(IGD_DEV,  PCI_BASE_ADDRESS_0);

There looks to be a stray blank in here.

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

Re: [Xen-devel] [PATCH v4 4/6] pci: switch pci_conf_write8 to use pci_sbdf_t

[Jan Beulich]

On 19.07.2019 16:07, Roger Pau Monne wrote:
> This reduces the number of parameters of the function to two, and
> simplifies some of the calling sites.
> 
> Signed-off-by: Roger Pau Monné 

Reviewed-by: Jan Beulich 

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

Re: [Xen-devel] [PATCH] x86/p2m: fix non-translated handling of iommu mappings

[Jan Beulich]

On 22.07.2019 17:32, Roger Pau Monne wrote:
> The current usage of need_iommu_pt_sync in p2m for non-translated
> guests is wrong because it doesn't correctly handle a relaxed PV
> hardware domain, that has need_sync set to false, but still need
> entries to be added from calls to {set/clear}_identity_p2m_entry.
> 
> Adjust the code in guest_physmap_add_page to also check whether the
> domain has an iommu instead of whether it needs syncing or not in
> order to take a reference to a page to be mapped.

Why this seemingly unrelated change? I ask because ...

> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -836,7 +836,7 @@ guest_physmap_add_page(struct domain *d, gfn_t gfn, mfn_t 
> mfn,
>*/
>   for ( i = 0; i < (1UL << page_order); ++i, ++page )
>   {
> -if ( !need_iommu_pt_sync(d) )
> +if ( !has_iommu_pt(d) )
>   /* nothing */;
>   else if ( get_page_and_type(page, d, PGT_writable_page) )
>   put_page_and_type(page);

... this parallels the code in iommu_hwdom_init(), which similarly
uses need_iommu_pt_sync() (and during the prior discussion you did
agree with Paul that it shouldn't be changed). IOW the patch for
now can have my R-b only with this change and the respective part
of the description dropped.

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

Re: [Xen-devel] [PATCH] x86/iommu: add comment regarding setting of need_sync

[Jan Beulich]

On 22.07.2019 17:45, Roger Pau Monne wrote:
> Clarify why relaxed hardware domains don't need iommu page-table
> syncing.
> 
> Signed-off-by: Roger Pau Monné 

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

Re: [Xen-devel] [PATCH 1/2] xen/sched: fix locking in restore_vcpu_affinity()

[Dario Faggioli]

On Tue, 2019-07-23 at 11:20 +0200, Juergen Gross wrote:
> Commit 0763cd2687897b55e7 ("xen/sched: don't disable scheduler on
> cpus
> during suspend") removed a lock in restore_vcpu_affinity() which
> needs
> to stay: cpumask_scratch_cpu() must be protected by the scheduler
> lock. 
>
And indeed I recall the thing looked weird, and bringing it up... but
then I also managed to convince myself that it wasn't a problem, which
apparently is. :-(

Sorry for having overlooked that.

> restore_vcpu_affinity() is being called by thaw_domains(), so
> with multiple domains in the system another domain might already be
> running and the scheduler might make use of cpumask_scratch_cpu()
> already.
> 
> Signed-off-by: Juergen Gross 
>
Reviewed-by: Dario Faggioli 

Thanks and Regards
-- 
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
---
<> (Raistlin Majere)



signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v4 3/6] pci: switch pci_conf_read32 to use pci_sbdf_t

[Roger Pau Monné]

On Tue, Jul 23, 2019 at 10:15:35AM +, Jan Beulich wrote:
> On 19.07.2019 16:07, Roger Pau Monne wrote:
> > This reduces the number of parameters of the function to two, and
> > simplifies some of the calling sites.
> > 
> > While there convert {IGD/IOH}_DEV to be a pci_sbdf_t itself instead of
> > a device number.
> > 
> > Signed-off-by: Roger Pau Monné 
> > Acked-by: Brian Woods 
> > Reviewed-by: Kevin Tian 
> 
> Acked-by: Jan Beulich 
> with one further remark (easily addressed while committing):
> 
> > @@ -128,9 +128,9 @@ static void __init map_igd_reg(void)
> >   if ( igd_reg_va )
> >   return;
> >   
> > -igd_mmio   = pci_conf_read32(0, 0, IGD_DEV, 0, PCI_BASE_ADDRESS_1);
> > +igd_mmio   = pci_conf_read32(IGD_DEV, PCI_BASE_ADDRESS_1);
> >   igd_mmio <<= 32;
> > -igd_mmio  += pci_conf_read32(0, 0, IGD_DEV, 0, PCI_BASE_ADDRESS_0);
> > +igd_mmio  += pci_conf_read32(IGD_DEV,  PCI_BASE_ADDRESS_0);
> 
> There looks to be a stray blank in here.

Good catch, please adjust on commit if you don't mind.

Thanks, Roger.

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

Re: [Xen-devel] [PATCH] xen/arm: remove unused dt_device_node parameter

[Julien Grall]

On 18/07/2019 14:18, Viktor Mitin wrote:

Hi Julien,


Hi,



I've checked latest Xen staging, the patch has not been integrated yet.
Please integrate the patch if no objections.


Done now. Sorry for the delay.

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH 1/2] xen/sched: fix locking in restore_vcpu_affinity()

[Juergen Gross]

On 23.07.19 14:08, Jan Beulich wrote:

On 23.07.2019 11:20, Juergen Gross wrote:

--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -708,6 +708,8 @@ void restore_vcpu_affinity(struct domain *d)
* set v->processor of each of their vCPUs to something that will
* make sense for the scheduler of the cpupool in which they are in.
*/
+lock = vcpu_schedule_lock_irq(v);
+
   cpumask_and(cpumask_scratch_cpu(cpu), v->cpu_hard_affinity,
   cpupool_domain_cpumask(d));
   if ( cpumask_empty(cpumask_scratch_cpu(cpu)) )
@@ -731,6 +733,9 @@ void restore_vcpu_affinity(struct domain *d)
   
   v->processor = cpumask_any(cpumask_scratch_cpu(cpu));
   
+spin_unlock_irq(lock);

+
+   /* v->processor might have changed, so reacquire the lock. */


Whoever commits this (perhaps me, later today) will want to replace
the hard tab here.


Oh, I'm sorry for that!

We really want checkpatch in Xen!


Juergen

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

Re: [Xen-devel] [PATCH 1/2] xen/sched: fix locking in restore_vcpu_affinity()

[Jan Beulich]

On 23.07.2019 11:20, Juergen Gross wrote:
> --- a/xen/common/schedule.c
> +++ b/xen/common/schedule.c
> @@ -708,6 +708,8 @@ void restore_vcpu_affinity(struct domain *d)
>* set v->processor of each of their vCPUs to something that will
>* make sense for the scheduler of the cpupool in which they are in.
>*/
> +lock = vcpu_schedule_lock_irq(v);
> +
>   cpumask_and(cpumask_scratch_cpu(cpu), v->cpu_hard_affinity,
>   cpupool_domain_cpumask(d));
>   if ( cpumask_empty(cpumask_scratch_cpu(cpu)) )
> @@ -731,6 +733,9 @@ void restore_vcpu_affinity(struct domain *d)
>   
>   v->processor = cpumask_any(cpumask_scratch_cpu(cpu));
>   
> +spin_unlock_irq(lock);
> +
> + /* v->processor might have changed, so reacquire the lock. */

Whoever commits this (perhaps me, later today) will want to replace
the hard tab here.

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

Re: [Xen-devel] [PATCH 2/2] xen: merge temporary vcpu pinning scenarios

[Andrew Cooper]

On 23/07/2019 10:20, Juergen Gross wrote:
> Today there are three scenarios which are pinning vcpus temporarily to
> a single physical cpu:
>
> - NMI/MCE injection into PV domains
> - wait_event() handling
> - vcpu_pin_override() handling
>
> Each of those cases are handled independently today using their own
> temporary cpumask to save the old affinity settings.
>
> The three cases can be combined as the two latter cases will only pin
> a vcpu to the physical cpu it is already running on, while
> vcpu_pin_override() is allowed to fail.
>
> So merge the three temporary pinning scenarios by only using one
> cpumask and a per-vcpu bitmask for specifying which of the three
> scenarios is currently active (they are allowed to nest).
>
> Note that we don't need to call domain_update_node_affinity() as we
> are only pinning for a brief period of time.
>
> Signed-off-by: Juergen Gross 
> ---
>  xen/arch/x86/pv/traps.c | 20 +---
>  xen/arch/x86/traps.c|  8 ++--
>  xen/common/domain.c |  4 +---
>  xen/common/schedule.c   | 35 +++
>  xen/common/wait.c   | 26 --
>  xen/include/xen/sched.h |  8 +---
>  6 files changed, 40 insertions(+), 61 deletions(-)
>
> diff --git a/xen/arch/x86/pv/traps.c b/xen/arch/x86/pv/traps.c
> index 1740784ff2..37dac300ba 100644
> --- a/xen/arch/x86/pv/traps.c
> +++ b/xen/arch/x86/pv/traps.c
> @@ -151,25 +151,7 @@ static void nmi_mce_softirq(void)
>  
>  BUG_ON(st->vcpu == NULL);
>  
> -/*
> - * Set the tmp value unconditionally, so that the check in the iret
> - * hypercall works.
> - */
> -cpumask_copy(st->vcpu->cpu_hard_affinity_tmp,
> - st->vcpu->cpu_hard_affinity);
> -
> -if ( (cpu != st->processor) ||
> - (st->processor != st->vcpu->processor) )
> -{
> -
> -/*
> - * We are on a different physical cpu.  Make sure to wakeup the vcpu 
> on
> - * the specified processor.
> - */
> -vcpu_set_hard_affinity(st->vcpu, cpumask_of(st->processor));
> -
> -/* Affinity is restored in the iret hypercall. */
> -}
> +vcpu_set_tmp_affinity(st->vcpu, st->processor, VCPU_AFFINITY_NMI);

Please can we keep the comment explaining where the affinity is
restored, which is a disguised explanation of why it is PV-only.

> diff --git a/xen/common/schedule.c b/xen/common/schedule.c
> index 89bc259ae4..d4de74f9c8 100644
> --- a/xen/common/schedule.c
> +++ b/xen/common/schedule.c
> @@ -1106,47 +1106,58 @@ void watchdog_domain_destroy(struct domain *d)
>  kill_timer(&d->watchdog_timer[i]);
>  }
>  
> -int vcpu_pin_override(struct vcpu *v, int cpu)
> +int vcpu_set_tmp_affinity(struct vcpu *v, int cpu, uint8_t reason)
>  {
>  spinlock_t *lock;
>  int ret = -EINVAL;
> +bool migrate;
>  
>  lock = vcpu_schedule_lock_irq(v);
>  
>  if ( cpu < 0 )
>  {
> -if ( v->affinity_broken )
> +if ( v->affinity_broken & reason )
>  {
> -sched_set_affinity(v, v->cpu_hard_affinity_saved, NULL);
> -v->affinity_broken = 0;
>  ret = 0;
> +v->affinity_broken &= ~reason;
>  }
> +if ( !ret && !v->affinity_broken )
> +sched_set_affinity(v, v->cpu_hard_affinity_saved, NULL);
>  }
>  else if ( cpu < nr_cpu_ids )
>  {
> -if ( v->affinity_broken )
> +if ( (v->affinity_broken & reason) ||
> + (v->affinity_broken && v->processor != cpu) )
>  ret = -EBUSY;
>  else if ( cpumask_test_cpu(cpu, VCPU2ONLINE(v)) )
>  {
> -cpumask_copy(v->cpu_hard_affinity_saved, v->cpu_hard_affinity);
> -v->affinity_broken = 1;
> -sched_set_affinity(v, cpumask_of(cpu), NULL);
> +if ( !v->affinity_broken )
> +{
> +cpumask_copy(v->cpu_hard_affinity_saved, 
> v->cpu_hard_affinity);
> +sched_set_affinity(v, cpumask_of(cpu), NULL);
> +}
> +v->affinity_broken |= reason;
>  ret = 0;
>  }
>  }
>  
> -if ( ret == 0 )
> +migrate = !ret && !cpumask_test_cpu(v->processor, v->cpu_hard_affinity);
> +if ( migrate )
>  vcpu_migrate_start(v);
>  
>  vcpu_schedule_unlock_irq(lock, v);
>  
> -domain_update_node_affinity(v->domain);
> -
> -vcpu_migrate_finish(v);
> +if ( migrate )
> +vcpu_migrate_finish(v);
>  
>  return ret;
>  }
>  
> +int vcpu_pin_override(struct vcpu *v, int cpu)

There are exactly two callers of vcpu_pin_override().  I'd take the
opportunity to make vcpu_set_tmp_affinity() the single API call for
adjusting affinity.

> +{
> +return vcpu_set_tmp_affinity(v, cpu, VCPU_AFFINITY_OVERRIDE);
> +}
> +
>  typedef long ret_t;
>  
>  #endif /* !COMPAT */
> diff --git a/xen/common/wait.c b/xen/common/wait.c
> index 4f830a14e8..9f9ad033b3 100644
> --- a/xen/common/wait.c
> +++ b/xen/common/wait.c

Re: [Xen-devel] [PATCH] x86/p2m: fix non-translated handling of iommu mappings

[Roger Pau Monné]

On Tue, Jul 23, 2019 at 10:32:41AM +, Jan Beulich wrote:
> On 22.07.2019 17:32, Roger Pau Monne wrote:
> > The current usage of need_iommu_pt_sync in p2m for non-translated
> > guests is wrong because it doesn't correctly handle a relaxed PV
> > hardware domain, that has need_sync set to false, but still need
> > entries to be added from calls to {set/clear}_identity_p2m_entry.
> > 
> > Adjust the code in guest_physmap_add_page to also check whether the
> > domain has an iommu instead of whether it needs syncing or not in
> > order to take a reference to a page to be mapped.
> 
> Why this seemingly unrelated change? I ask because ...
> 
> > --- a/xen/arch/x86/mm/p2m.c
> > +++ b/xen/arch/x86/mm/p2m.c
> > @@ -836,7 +836,7 @@ guest_physmap_add_page(struct domain *d, gfn_t gfn, 
> > mfn_t mfn,
> >*/
> >   for ( i = 0; i < (1UL << page_order); ++i, ++page )
> >   {
> > -if ( !need_iommu_pt_sync(d) )
> > +if ( !has_iommu_pt(d) )
> >   /* nothing */;
> >   else if ( get_page_and_type(page, d, PGT_writable_page) )
> >   put_page_and_type(page);
> 
> ... this parallels the code in iommu_hwdom_init(), which similarly
> uses need_iommu_pt_sync() (and during the prior discussion you did
> agree with Paul that it shouldn't be changed). IOW the patch for
> now can have my R-b only with this change and the respective part
> of the description dropped.

OK, this is again not needed for a relaxed PV hardware domain because
all RAM is already mapped on the iommu page tables.

Let me update the commit and resend.

Thanks, Roger.

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

[Xen-devel] [PATCH v2] x86/p2m: fix non-translated handling of iommu mappings

[Roger Pau Monne]

The current usage of need_iommu_pt_sync in p2m for non-translated
guests is wrong because it doesn't correctly handle a relaxed PV
hardware domain, that has need_sync set to false, but still need
entries to be added from calls to {set/clear}_identity_p2m_entry.

Signed-off-by: Roger Pau Monné 
Reviewed-by: Paul Durrant 
---
Cc: George Dunlap 
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Wei Liu 
Cc: Paul Durrant 
---
Changes since v1:
 - Drop the guest_physmap_add_page modification.
---
 xen/arch/x86/mm/p2m.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index fef97c82f6..3605614aaf 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1341,7 +1341,7 @@ int set_identity_p2m_entry(struct domain *d, unsigned 
long gfn_l,
 
 if ( !paging_mode_translate(p2m->domain) )
 {
-if ( !need_iommu_pt_sync(d) )
+if ( !has_iommu_pt(d) )
 return 0;
 return iommu_legacy_map(d, _dfn(gfn_l), _mfn(gfn_l), PAGE_ORDER_4K,
 IOMMUF_readable | IOMMUF_writable);
@@ -1432,7 +1432,7 @@ int clear_identity_p2m_entry(struct domain *d, unsigned 
long gfn_l)
 
 if ( !paging_mode_translate(d) )
 {
-if ( !need_iommu_pt_sync(d) )
+if ( !has_iommu_pt(d) )
 return 0;
 return iommu_legacy_unmap(d, _dfn(gfn_l), PAGE_ORDER_4K);
 }
-- 
2.20.1 (Apple Git-117)


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

Re: [Xen-devel] [PATCH 2/2] xen: merge temporary vcpu pinning scenarios

[Jan Beulich]

On 23.07.2019 11:20, Juergen Gross wrote:
> Today there are three scenarios which are pinning vcpus temporarily to
> a single physical cpu:
> 
> - NMI/MCE injection into PV domains
> - wait_event() handling
> - vcpu_pin_override() handling
> 
> Each of those cases are handled independently today using their own
> temporary cpumask to save the old affinity settings.

And what guarantees that no two of them will be in use at the same time?
You don't even mention ...

> The three cases can be combined as the two latter cases will only pin
> a vcpu to the physical cpu it is already running on, while
> vcpu_pin_override() is allowed to fail.

.. the NMI/#MC injection case here (despite the use of "the two" and
"while" giving the impression). Or (looking at the actual code) did you
mean "former" instead of "latter"? But if so - id that true?
v->processor gets latched into st->processor before raising the softirq,
but can't the vCPU be moved elsewhere by the time the softirq handler
actually gains control? If that's not possible (and if it's not obvious
why, and as you can see it's not obvious to me), then I think a code
comment wants to be added there.

Independent of that introducing new failure cases for vcpu_pin_override()
isn't really nice. Then again any two racing/conflicting pinning attempts
can't result in anything good.

Nevertheless - nice idea, so a few minor comments on the code as well, in
the hope that my point above can be addressed.

> --- a/xen/common/schedule.c
> +++ b/xen/common/schedule.c
> @@ -1106,47 +1106,58 @@ void watchdog_domain_destroy(struct domain *d)
>   kill_timer(&d->watchdog_timer[i]);
>   }
>   
> -int vcpu_pin_override(struct vcpu *v, int cpu)
> +int vcpu_set_tmp_affinity(struct vcpu *v, int cpu, uint8_t reason)

I'd find it pretty nice if at this occasion the type of "cpu" was
changed to "unsigned int", using UINT_MAX or NR_CPUS instead of -1
for the "remove override" case.

I'd also prefer if you didn't use "tmp" as an infix here. Make it
"temporary", "transient", or some such. Perhaps even omit "set",
the more that the function may also clear it.

> @@ -182,30 +178,24 @@ static void __prepare_to_wait(struct waitqueue_vcpu 
> *wqv)
>   static void __finish_wait(struct waitqueue_vcpu *wqv)
>   {
>   wqv->esp = NULL;
> -(void)vcpu_set_hard_affinity(current, &wqv->saved_affinity);
> +vcpu_set_tmp_affinity(current, -1, VCPU_AFFINITY_WAIT);
>   }
>   
>   void check_wakeup_from_wait(void)
>   {
> -struct waitqueue_vcpu *wqv = current->waitqueue_vcpu;
> +struct vcpu *curr = current;
> +struct waitqueue_vcpu *wqv = curr->waitqueue_vcpu;
>   
>   ASSERT(list_empty(&wqv->list));
>   
>   if ( likely(wqv->esp == NULL) )
>   return;
>   
> -/* Check if we woke up on the wrong CPU. */
> -if ( unlikely(smp_processor_id() != wqv->wakeup_cpu) )
> +/* Check if we are still pinned. */
> +if ( unlikely(!(curr->affinity_broken & VCPU_AFFINITY_WAIT)) )
>   {
> -/* Re-set VCPU affinity and re-enter the scheduler. */
> -struct vcpu *curr = current;
> -cpumask_copy(&wqv->saved_affinity, curr->cpu_hard_affinity);
> -if ( vcpu_set_hard_affinity(curr, cpumask_of(wqv->wakeup_cpu)) )
> -{
> -gdprintk(XENLOG_ERR, "Unable to set vcpu affinity\n");
> -domain_crash(current->domain);
> -}
> -wait(); /* takes us back into the scheduler */
> +gdprintk(XENLOG_ERR, "vcpu affinity lost\n");
> +domain_crash(current->domain);
>   }

Please use curr in favor of current.

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

Re: [Xen-devel] [PATCH v2] x86/p2m: fix non-translated handling of iommu mappings

[Jan Beulich]

On 23.07.2019 14:43, Roger Pau Monne wrote:
> The current usage of need_iommu_pt_sync in p2m for non-translated
> guests is wrong because it doesn't correctly handle a relaxed PV
> hardware domain, that has need_sync set to false, but still need
> entries to be added from calls to {set/clear}_identity_p2m_entry.
> 
> Signed-off-by: Roger Pau Monné 
> Reviewed-by: Paul Durrant 

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

Re: [Xen-devel] [PATCH 1/2] xen/sched: fix locking in restore_vcpu_affinity()

[Andrew Cooper]

On 23/07/2019 13:08, Jan Beulich wrote:
> On 23.07.2019 11:20, Juergen Gross wrote:
>> --- a/xen/common/schedule.c
>> +++ b/xen/common/schedule.c
>> @@ -708,6 +708,8 @@ void restore_vcpu_affinity(struct domain *d)
>>* set v->processor of each of their vCPUs to something that will
>>* make sense for the scheduler of the cpupool in which they are 
>> in.
>>*/
>> +lock = vcpu_schedule_lock_irq(v);
>> +
>>   cpumask_and(cpumask_scratch_cpu(cpu), v->cpu_hard_affinity,
>>   cpupool_domain_cpumask(d));
>>   if ( cpumask_empty(cpumask_scratch_cpu(cpu)) )
>> @@ -731,6 +733,9 @@ void restore_vcpu_affinity(struct domain *d)
>>   
>>   v->processor = cpumask_any(cpumask_scratch_cpu(cpu));
>>   
>> +spin_unlock_irq(lock);
>> +
>> +/* v->processor might have changed, so reacquire the lock. */
> Whoever commits this (perhaps me, later today) will want to replace
> the hard tab here.

I've already committed this, and did fix up the tab.

~Andrew

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

Re: [Xen-devel] [PATCH 2/2] xen: merge temporary vcpu pinning scenarios

[Dario Faggioli]

On Tue, 2019-07-23 at 11:20 +0200, Juergen Gross wrote:
> Today there are three scenarios which are pinning vcpus temporarily
> to
> a single physical cpu:
> 
> - NMI/MCE injection into PV domains
> - wait_event() handling
> - vcpu_pin_override() handling
> 
> Each of those cases are handled independently today using their own
> temporary cpumask to save the old affinity settings.
>
> The three cases can be combined as the two latter cases will only pin
> a vcpu to the physical cpu it is already running on, while
> vcpu_pin_override() is allowed to fail.
> 
> So merge the three temporary pinning scenarios by only using one
> cpumask and a per-vcpu bitmask for specifying which of the three
> scenarios is currently active (they are allowed to nest).
>
Right. And, after this patch, all the three cases above and
suspend/resume, all use cpu_hard_affinity_saved.

Can you add a paragraph, either here in the changelog, on in a comment
(e.g., at the top or inside of vcpu_set_tmp_affinity()), about why this
is all fine?

Regards
-- 
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
---
<> (Raistlin Majere)



signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 2/2] xen: merge temporary vcpu pinning scenarios

[Dario Faggioli]

On Tue, 2019-07-23 at 12:42 +, Jan Beulich wrote:
> On 23.07.2019 11:20, Juergen Gross wrote:
> > Today there are three scenarios which are pinning vcpus temporarily
> > to
> > a single physical cpu:
> > 
> > - NMI/MCE injection into PV domains
> > - wait_event() handling
> > - vcpu_pin_override() handling
> > 
> > Each of those cases are handled independently today using their own
> > temporary cpumask to save the old affinity settings.
> 
> And what guarantees that no two of them will be in use at the same
> time?
> You don't even mention ...
> 
AFAIUI, the case of any two of the above being in use at the same time
is handled. Basically, it is fine, as far as they (temporarily) set the
affinity to the same CPU, which is said to always be the case, here...

> > The three cases can be combined as the two latter cases will only
> > pin
> > a vcpu to the physical cpu it is already running on, while
> > vcpu_pin_override() is allowed to fail.
> 
...for the first two. For vcpu_pin_override(), if it is called for
pinning the vcpu to CPU X, with any of the other twos in effect, and
having temporarily pinned the vcpu to CPU X already, then things are
fine again. If vcpu_pin_override() wants to pin to CPU Y, it fails.

All that being said, I agree it'd be good to have a bit more comments
(and I asked the same, although about a different path, in my own
reply).

Regards
-- 
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
---
<> (Raistlin Majere)



signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 2/2] xen: merge temporary vcpu pinning scenarios

[Juergen Gross]

On 23.07.19 14:26, Andrew Cooper wrote:

On 23/07/2019 10:20, Juergen Gross wrote:

Today there are three scenarios which are pinning vcpus temporarily to
a single physical cpu:

- NMI/MCE injection into PV domains
- wait_event() handling
- vcpu_pin_override() handling

Each of those cases are handled independently today using their own
temporary cpumask to save the old affinity settings.

The three cases can be combined as the two latter cases will only pin
a vcpu to the physical cpu it is already running on, while
vcpu_pin_override() is allowed to fail.

So merge the three temporary pinning scenarios by only using one
cpumask and a per-vcpu bitmask for specifying which of the three
scenarios is currently active (they are allowed to nest).

Note that we don't need to call domain_update_node_affinity() as we
are only pinning for a brief period of time.

Signed-off-by: Juergen Gross 
---
  xen/arch/x86/pv/traps.c | 20 +---
  xen/arch/x86/traps.c|  8 ++--
  xen/common/domain.c |  4 +---
  xen/common/schedule.c   | 35 +++
  xen/common/wait.c   | 26 --
  xen/include/xen/sched.h |  8 +---
  6 files changed, 40 insertions(+), 61 deletions(-)

diff --git a/xen/arch/x86/pv/traps.c b/xen/arch/x86/pv/traps.c
index 1740784ff2..37dac300ba 100644
--- a/xen/arch/x86/pv/traps.c
+++ b/xen/arch/x86/pv/traps.c
@@ -151,25 +151,7 @@ static void nmi_mce_softirq(void)
  
  BUG_ON(st->vcpu == NULL);
  
-/*

- * Set the tmp value unconditionally, so that the check in the iret
- * hypercall works.
- */
-cpumask_copy(st->vcpu->cpu_hard_affinity_tmp,
- st->vcpu->cpu_hard_affinity);
-
-if ( (cpu != st->processor) ||
- (st->processor != st->vcpu->processor) )
-{
-
-/*
- * We are on a different physical cpu.  Make sure to wakeup the vcpu on
- * the specified processor.
- */
-vcpu_set_hard_affinity(st->vcpu, cpumask_of(st->processor));
-
-/* Affinity is restored in the iret hypercall. */
-}
+vcpu_set_tmp_affinity(st->vcpu, st->processor, VCPU_AFFINITY_NMI);


Please can we keep the comment explaining where the affinity is
restored, which is a disguised explanation of why it is PV-only.


Okay.




diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index 89bc259ae4..d4de74f9c8 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -1106,47 +1106,58 @@ void watchdog_domain_destroy(struct domain *d)
  kill_timer(&d->watchdog_timer[i]);
  }
  
-int vcpu_pin_override(struct vcpu *v, int cpu)

+int vcpu_set_tmp_affinity(struct vcpu *v, int cpu, uint8_t reason)
  {
  spinlock_t *lock;
  int ret = -EINVAL;
+bool migrate;
  
  lock = vcpu_schedule_lock_irq(v);
  
  if ( cpu < 0 )

  {
-if ( v->affinity_broken )
+if ( v->affinity_broken & reason )
  {
-sched_set_affinity(v, v->cpu_hard_affinity_saved, NULL);
-v->affinity_broken = 0;
  ret = 0;
+v->affinity_broken &= ~reason;
  }
+if ( !ret && !v->affinity_broken )
+sched_set_affinity(v, v->cpu_hard_affinity_saved, NULL);
  }
  else if ( cpu < nr_cpu_ids )
  {
-if ( v->affinity_broken )
+if ( (v->affinity_broken & reason) ||
+ (v->affinity_broken && v->processor != cpu) )
  ret = -EBUSY;
  else if ( cpumask_test_cpu(cpu, VCPU2ONLINE(v)) )
  {
-cpumask_copy(v->cpu_hard_affinity_saved, v->cpu_hard_affinity);
-v->affinity_broken = 1;
-sched_set_affinity(v, cpumask_of(cpu), NULL);
+if ( !v->affinity_broken )
+{
+cpumask_copy(v->cpu_hard_affinity_saved, v->cpu_hard_affinity);
+sched_set_affinity(v, cpumask_of(cpu), NULL);
+}
+v->affinity_broken |= reason;
  ret = 0;
  }
  }
  
-if ( ret == 0 )

+migrate = !ret && !cpumask_test_cpu(v->processor, v->cpu_hard_affinity);
+if ( migrate )
  vcpu_migrate_start(v);
  
  vcpu_schedule_unlock_irq(lock, v);
  
-domain_update_node_affinity(v->domain);

-
-vcpu_migrate_finish(v);
+if ( migrate )
+vcpu_migrate_finish(v);
  
  return ret;

  }
  
+int vcpu_pin_override(struct vcpu *v, int cpu)


There are exactly two callers of vcpu_pin_override().  I'd take the
opportunity to make vcpu_set_tmp_affinity() the single API call for
adjusting affinity.


Fine with me.




+{
+return vcpu_set_tmp_affinity(v, cpu, VCPU_AFFINITY_OVERRIDE);
+}
+
  typedef long ret_t;
  
  #endif /* !COMPAT */

diff --git a/xen/common/wait.c b/xen/common/wait.c
index 4f830a14e8..9f9ad033b3 100644
--- a/xen/common/wait.c
+++ b/xen/common/wait.c
@@ -182,30 +178,24 @@ static void __prepare_to_wait(struct waitqueue_vcpu *wqv)
  static void __finish_wait(struct waitqueue_vcpu *wqv)
  {
  w

Re: [Xen-devel] [PATCH 2/2] xen: merge temporary vcpu pinning scenarios

[Andrew Cooper]

On 23/07/2019 14:25, Juergen Gross wrote:
> On 23.07.19 14:26, Andrew Cooper wrote:
>> On 23/07/2019 10:20, Juergen Gross wrote:
>>
>>>   }
>>>     /*
>>> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
>>> index b40c8fd138..721c429454 100644
>>> --- a/xen/include/xen/sched.h
>>> +++ b/xen/include/xen/sched.h
>>> @@ -200,7 +200,10 @@ struct vcpu
>>>   /* VCPU is paused following shutdown request
>>> (d->is_shutting_down)? */
>>>   bool paused_for_shutdown;
>>>   /* VCPU need affinity restored */
>>> -    bool affinity_broken;
>>> +    uint8_t  affinity_broken;
>>> +#define VCPU_AFFINITY_OVERRIDE    0x01
>>> +#define VCPU_AFFINITY_NMI 0x02
>>
>> VCPU_AFFINITY_NMI_MCE ?  It is used for more than just NMIs.
>
> Okay.
>
> BTW: The MCE case is never triggered (there is no caller of
> pv_raise_interrupt() with TRAP_machine_check). Is this code left in
> place on purpose or could it be removed?

It come from the restore_all_guest path in assembly, via process_mce.

This code is horribly tangled and in severe need of rewriting in C.

~Andrew

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

Re: [Xen-devel] [PATCH V1] iommu/arm: Add Renesas IPMMU-VMSA support

[Julien Grall]

Hi Oleksandr,

On 22/07/2019 17:27, Oleksandr wrote:

On 20.07.19 21:25, Julien Grall wrote:

Apologies for the late answer. I have been traveling for the past few weeks.


Absolutely no problem. Thank you for your review.




On 6/26/19 11:30 AM, Oleksandr Tyshchenko wrote:

From: Oleksandr Tyshchenko 

The IPMMU-VMSA is VMSA-compatible I/O Memory Management Unit (IOMMU)
which provides address translation and access protection functionalities
to processing units and interconnect networks.


Do you have a link to the specification?


All I have is a TRM. Unfortunately, I can't share it.


Does anyone in the community has access to the spec?




+  hardware supports stage 2 translation table format and is able to use
+  CPU's P2M table as is.
+
  endif
diff --git a/xen/drivers/passthrough/arm/Makefile 
b/xen/drivers/passthrough/arm/Makefile

index b3efcfd..40ac7a9 100644
--- a/xen/drivers/passthrough/arm/Makefile
+++ b/xen/drivers/passthrough/arm/Makefile
@@ -1,2 +1,3 @@
  obj-y += iommu.o
  obj-$(CONFIG_ARM_SMMU) += smmu.o
+obj-$(CONFIG_IPMMU_VMSA) += ipmmu-vmsa.o
diff --git a/xen/drivers/passthrough/arm/ipmmu-vmsa.c 
b/xen/drivers/passthrough/arm/ipmmu-vmsa.c

new file mode 100644
index 000..5091c61
--- /dev/null
+++ b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
@@ -0,0 +1,1487 @@
+/*
+ * xen/drivers/passthrough/arm/ipmmu-vmsa.c
+ *
+ * Driver for the Renesas IPMMU-VMSA found in R-Car Gen3 SoCs.
+ *
+ * The IPMMU-VMSA is VMSA-compatible I/O Memory Management Unit (IOMMU)
+ * which provides address translation and access protection functionalities
+ * to processing units and interconnect networks.
+ *
+ * Please note, current driver is supposed to work only with newest Gen3 SoCs
+ * revisions which IPMMU hardware supports stage 2 translation table format and
+ * is able to use CPU's P2M table as is.
+ *
+ * Based on Linux's IPMMU-VMSA driver from Renesas BSP:
+ *    drivers/iommu/ipmmu-vmsa.c


What are the major differences compare the Linux driver?


Well, the major differences are:

1. Stage 1/Stage 2 translation. Linux driver supports Stage 1 translation only 
(with Stage 1 translation table format). It manages page table by itself. But 
Xen driver supports Stage 2 translation (with Stage 2 translation table format) 
to be able to share the page table with the CPU. Stage 1 translation is always 
bypassed in Xen driver.


So, Xen driver is supposed to be used with newest Gen3 SoC revisions only (H3 
ES3.0, M3 ES3.0, etc.) which IPMMU hardware does support stage 2 translation 
table format.


2. AArch64 support. Linux driver uses VMSAv8-32 mode, while Xen driver enables 
Armv8 VMSAv8-64 mode to cover up to 40 bit input address.


3. Context bank (sets of page table) usage. In Xen, each context bank is mapped 
to one Xen domain.  So, all devices being pass throughed to the same Xen domain 
share the same context bank.


Can this be written in the commit message? This is helpful for anyone reviewing 
the driver today and future developer.









+ * you can found at:
+ *    url: git://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git
+ *    branch: v4.14.75-ltsi/rcar-3.9.2
+ *    commit: a5266d298124874c2c06b8b13d073f6ecc2ee355


Is there any reason to use the BSP driver and not the one provided by Linux 
directly?


I was thinking the BSP driver is a *little bit* more updated than Linux one. 
Sometime it was a big difference between mainline and BSP driver. But now


the difference is not big and mostly in DDR_BACKUP and WHITELIST support. I 
looked at mainline driver as well when implementing Xen driver.


What is the review process for patches to be merged in the BSP? Is it the same 
as Linux upstream?







[...]


+#define dev_archdata(dev) ((struct ipmmu_vmsa_xen_device *)dev->archdata.iommu)
+
+/* Root/Cache IPMMU device's information */
+struct ipmmu_vmsa_device
+{


AFAICT, this file was converted to Xen coding style. If so, the style for 
struct is


struct ... {
...
};


Yes, will correct everywhere in this file.





+    struct device *dev;
+    void __iomem *base;
+    struct ipmmu_vmsa_device *root;
+    struct list_head list;
+    unsigned int num_utlbs;
+    unsigned int num_ctx;
+    spinlock_t lock;    /* Protects ctx and domains[] */
+    DECLARE_BITMAP(ctx, IPMMU_CTX_MAX);
+    struct ipmmu_vmsa_domain *domains[IPMMU_CTX_MAX];
+};
+
+/*
+ * Root/Cache IPMMU domain's information
+ *
+ * Root IPMMU device is assigned to Root IPMMU domain while Cache IPMMU device
+ * is assigned to Cache IPMMU domain. Master devices are connected to Cache
+ * IPMMU devices through specific ports called micro-TLBs.
+ * All Cache IPMMU devices, in turn, are connected to Root IPMMU device
+ * which manages IPMMU context.
+ */
+struct ipmmu_vmsa_domain
+{


Ditto.


ok





+    /*
+ * IPMMU device assigned to this IPMMU domain.
+ * Either Root device which is located at the main memory bus domain or
+ * Cache device which is located at each hierarchy bus domain.
+ */

Re: [Xen-devel] [PATCH] docs/sphinx: todo/wishlist

[Andrew Cooper]

On 23/07/2019 05:36, Juergen Gross wrote:
> On 22.07.19 21:20, Andrew Cooper wrote:
>> diff --git a/docs/misc/wishlist.rst b/docs/misc/wishlist.rst
>> new file mode 100644
>> index 00..6cdb47d6e7
>> --- /dev/null
>> +++ b/docs/misc/wishlist.rst
>> @@ -0,0 +1,53 @@
>> +Development Wishlist
>> +
>> +
>> +Remove xenstored's dependencies on unstable interfaces
>> +--
>> +
>> +Various xenstored implementations use libxc for two purposes.  It
>> would be a
>> +substantial advantage to move xenstored onto entirely stable
>> interfaces, which
>> +disconnects it from the internal of the libxc.
>> +
>> +1. Foreign mapping of the store ring
>> +
>> +
>> +This is obsolete since :xen-cs:`6a2de353a9` (2012) which allocated
>> grant
>> +entries instead, to allow xenstored to function as a stub-domain
>> without dom0
>> +permissions.  :xen-cs:`38eeb3864d` dropped foreign mapping for
>> cxenstored.
>> +However, there are no OCaml bindings for libxengnttab.
>> +
>> +Work Items:
>> +
>> +* Minimal ``tools/ocaml/libs/xg/`` binding for ``tools/libs/gnttab/``.
>> +* Replicate :xen-cs:`38eeb3864d` for oxenstored as well.
>> +
>> +
>> +2. Figuring out which domain(s) have gone away
>> +~~
>> +
>> +Currently, the handling of domains is asymmetric.
>> +
>> +* When a domain is created, the toolstack explicitly sends an
>> +  ``XS_INTRODUCE(domid, store mfn, store evtchn)`` message to
>> xenstored, to
>> +  cause xenstored to connect to the guest ring, and fire the
>> +  ``@introduceDomain`` watch.
>> +* When a domain is destroyed, Xen fires ``VIRQ_DOM_EXC`` which is
>> bound by
>> +  xenstored, rather than the toolstack.  xenstored updates its idea
>> of the
>> +  status of domains, and fires the ``@releaseDomain`` watch.
>> +
>> +Xenstored uses ``xc_domain_getinfo()``, to work out which domain(s)
>> have gone
>> +away, and only cares about the shutdown status.
>> +
>> +Furthermore, ``@releaseDomain`` (like ``VIRQ_DOM_EXC``) is a single-bit
>> +message, which requires all listeners to evaluate whether the
>> message applies
>> +to them or not.  This results in a flurry of ``xc_domain_getinfo()``
>> calls
>> +from multiple entities in the system, which all serialise on the
>> domctl lock
>> +in Xen.
>> +
>> +Work Items:
>> +
>> +* Figure out how shutdown status can be expressed in a stable way
>> from Xen.
>> +* Figure out if ``VIRQ_DOM_EXC`` and ``@releaseDomain`` can be
>> extended to
>> +  carry at least a domid, to make domain shutdown scale better.
>
> @releaseDomain (and @introduceDomain) can't be extended, we'd need to
> add another watch path like @domainStatus//. Xenstored
> could advertise its capability to raise this watch in /tool/xenstored.

I guess I was being a bit fast and loose with terminology.  I didn't
intend to imply "literally modify @{introduce,release}Domain", as they
are already fixed ABIs, but more to "compatibly build something which is
better".

That scheme would work for improved @releaseDomain, but it wouldn't work
for an improved introduce.  Introduce needs a single key to watch on,
which hands back the domid so you don't need to go searching for it.

>
> As VIRQ_DOM_EXC is just an event I don't see how the domid could be
> passed by it. I guess we'd need e.g. a shared memory area which the
> domain registered for VIRQ_DOM_EXC could map and which would contain a
> bitmap (one bit per domain). The hypervisor would set the bit on a
> status change and fire VIRQ_DOM_EXC, xenstored would look for a set
> bit, clear it and read the status of the related domain.

The point here is to avoid using xc_domain_getinfo() in the first place,
so there needs to be no "getting the status of the domain".

DOM_EXC is fired for domain_shutdown() only (but for reasons which
escape me, fired twice).  Given that a domid is a 15 bit number, a
bitmap of all domains does fit within a single 4k page.

All xenstored needs to know is that the domain shut down - nothing
else.  Given a mapping such as the above, xenstored, on getting
VIRQ_DOM_EXC would memcpy the page sideways, then loop over the page
looking for bits which have become set, notify for each of those
domains, and clear the bit in the page.

>
>> +* Figure out if ``VIRQ_DOM_EXC`` would better be bound by the
>> toolstack,
>> +  rather than xenstored.
>
> No, I don't think so. This would need another daemon.

There is exactly one toolstack which is nominally daemonless, and it
doesn't account for any substantial proportion of the Xen install-base,
worldwide.

The fact xl might not cope has no relevance to the technical discussion
of whether the current asymmetric setup is sane or not, and whether it
can, in principle, be improved.

~Andrew

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

Re: [Xen-devel] [PATCH 2/2] xen: merge temporary vcpu pinning scenarios

[Juergen Gross]

On 23.07.19 14:42, Jan Beulich wrote:

On 23.07.2019 11:20, Juergen Gross wrote:

Today there are three scenarios which are pinning vcpus temporarily to
a single physical cpu:

- NMI/MCE injection into PV domains
- wait_event() handling
- vcpu_pin_override() handling

Each of those cases are handled independently today using their own
temporary cpumask to save the old affinity settings.


And what guarantees that no two of them will be in use at the same time?
You don't even mention ...


The three cases can be combined as the two latter cases will only pin
a vcpu to the physical cpu it is already running on, while
vcpu_pin_override() is allowed to fail.


.. the NMI/#MC injection case here (despite the use of "the two" and
"while" giving the impression). Or (looking at the actual code) did you
mean "former" instead of "latter"? But if so - id that true?


Yes, I meant former.


v->processor gets latched into st->processor before raising the softirq,
but can't the vCPU be moved elsewhere by the time the softirq handler
actually gains control? If that's not possible (and if it's not obvious
why, and as you can see it's not obvious to me), then I think a code
comment wants to be added there.


You are right, it might be possible for the vcpu to move around.

OTOH is it really important to run the target vcpu exactly on the cpu
it is executing (or has last executed) at the time the NMI/MCE is being
queued? This is in no way related to the cpu the MCE or NMI has been
happening on. It is just a random cpu, and so it would be if we'd do the
cpu selection when the softirq handler is running.

One question to understand the idea nehind all that: _why_ is the vcpu
pinned until it does an iret? I could understand if it would be pinned
to the cpu where the NMI/MCE was happening, but this is not the case.


Independent of that introducing new failure cases for vcpu_pin_override()
isn't really nice. Then again any two racing/conflicting pinning attempts
can't result in anything good.


Right.


Nevertheless - nice idea, so a few minor comments on the code as well, in
the hope that my point above can be addressed.


--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -1106,47 +1106,58 @@ void watchdog_domain_destroy(struct domain *d)
   kill_timer(&d->watchdog_timer[i]);
   }
   
-int vcpu_pin_override(struct vcpu *v, int cpu)

+int vcpu_set_tmp_affinity(struct vcpu *v, int cpu, uint8_t reason)


I'd find it pretty nice if at this occasion the type of "cpu" was
changed to "unsigned int", using UINT_MAX or NR_CPUS instead of -1
for the "remove override" case.


Fine with me.


I'd also prefer if you didn't use "tmp" as an infix here. Make it
"temporary", "transient", or some such. Perhaps even omit "set",
the more that the function may also clear it.


Okay.




@@ -182,30 +178,24 @@ static void __prepare_to_wait(struct waitqueue_vcpu *wqv)
   static void __finish_wait(struct waitqueue_vcpu *wqv)
   {
   wqv->esp = NULL;
-(void)vcpu_set_hard_affinity(current, &wqv->saved_affinity);
+vcpu_set_tmp_affinity(current, -1, VCPU_AFFINITY_WAIT);
   }
   
   void check_wakeup_from_wait(void)

   {
-struct waitqueue_vcpu *wqv = current->waitqueue_vcpu;
+struct vcpu *curr = current;
+struct waitqueue_vcpu *wqv = curr->waitqueue_vcpu;
   
   ASSERT(list_empty(&wqv->list));
   
   if ( likely(wqv->esp == NULL) )

   return;
   
-/* Check if we woke up on the wrong CPU. */

-if ( unlikely(smp_processor_id() != wqv->wakeup_cpu) )
+/* Check if we are still pinned. */
+if ( unlikely(!(curr->affinity_broken & VCPU_AFFINITY_WAIT)) )
   {
-/* Re-set VCPU affinity and re-enter the scheduler. */
-struct vcpu *curr = current;
-cpumask_copy(&wqv->saved_affinity, curr->cpu_hard_affinity);
-if ( vcpu_set_hard_affinity(curr, cpumask_of(wqv->wakeup_cpu)) )
-{
-gdprintk(XENLOG_ERR, "Unable to set vcpu affinity\n");
-domain_crash(current->domain);
-}
-wait(); /* takes us back into the scheduler */
+gdprintk(XENLOG_ERR, "vcpu affinity lost\n");
+domain_crash(current->domain);
   }


Please use curr in favor of current.


Yes.


Juergen

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

Re: [Xen-devel] [PATCH] docs/sphinx: todo/wishlist

[Andrew Cooper]

On 22/07/2019 20:20, Andrew Cooper wrote:
> a.k.a. (at least in this form) Andrew's "work which might be offloadable to
> someone else" list.
>
> Signed-off-by: Andrew Cooper 
> ---
> CC: George Dunlap 
> CC: Ian Jackson 
> CC: Jan Beulich 
> CC: Stefano Stabellini 
> CC: Tim Deegan 
> CC: Wei Liu 
> CC: Julien Grall 
> CC: Lars Kurth 
> CC: Paul Durrant 
> CC: Roger Pau Monné 
>
> RFC for obvious reasons.
>
> A rendered version of this can be found at:
> https://andrewcoop-xen.readthedocs.io/en/docs-wishlist/misc/wishlist.html
>
> During XenSummit in Chicago, it was expressed several times that having some
> todo lists would be a benefit, to help coordinate work in related areas.
>
> Here is an attempt to start one.  For now, it covers one single
> item (xenstored's use of non-stable APIs) to get some feedback about the
> general approach.  I have plenty to get stuck into in Xen itself if this way
> of expressing them isn't deemed unacceptable.
>
> As for the wishlist itself, I think it is important that it be restricted to
> concrete actions (i.e. already partially groomed, if you speak agile), which
> are identified problems, and suggested fixes.
>
> In particular, I don't think it is appropriate to devolve into a bullet point
> list of new features, or tasks like "document $whotsit".  It should be
> restricted to things which are real problems, on existing systems, which have
> some forward plan of action.  That way, any developer should be able to
> cross-reference at least at a high level, and see if there are areas of
> overlapping work, or whether a slightly tweaked approach might be suitable for
> multiple areas.
>
> Anyway - thoughts from the peanut gallery?
> ---
>  docs/conf.py   | 10 +-
>  docs/index.rst |  9 +
>  docs/misc/wishlist.rst | 53 
> ++

It has been pointed out that calling this the wishlist is a poor name. 
A better name would be the technical debt list.  I won't resend for just
this, but please bear it in mind when considering the suggestion.

~Andrew

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

Re: [Xen-devel] [PATCH 2/2] xen: merge temporary vcpu pinning scenarios

[Jan Beulich]

On 23.07.2019 15:44, Juergen Gross wrote:
> On 23.07.19 14:42, Jan Beulich wrote:
>> v->processor gets latched into st->processor before raising the softirq,
>> but can't the vCPU be moved elsewhere by the time the softirq handler
>> actually gains control? If that's not possible (and if it's not obvious
>> why, and as you can see it's not obvious to me), then I think a code
>> comment wants to be added there.
> 
> You are right, it might be possible for the vcpu to move around.
> 
> OTOH is it really important to run the target vcpu exactly on the cpu
> it is executing (or has last executed) at the time the NMI/MCE is being
> queued? This is in no way related to the cpu the MCE or NMI has been
> happening on. It is just a random cpu, and so it would be if we'd do the
> cpu selection when the softirq handler is running.
> 
> One question to understand the idea nehind all that: _why_ is the vcpu
> pinned until it does an iret? I could understand if it would be pinned
> to the cpu where the NMI/MCE was happening, but this is not the case.

Then it was never finished or got broken, I would guess. Especially for
#MC the idea is/was for the domain to be able to access the MSRs in
question (). Iirc, that is, and prior to vMCE appearing.

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

Re: [Xen-devel] [PATCH 2/2] xen: merge temporary vcpu pinning scenarios

[Juergen Gross]

On 23.07.19 15:31, Andrew Cooper wrote:

On 23/07/2019 14:25, Juergen Gross wrote:

On 23.07.19 14:26, Andrew Cooper wrote:

On 23/07/2019 10:20, Juergen Gross wrote:


   }
     /*
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index b40c8fd138..721c429454 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -200,7 +200,10 @@ struct vcpu
   /* VCPU is paused following shutdown request
(d->is_shutting_down)? */
   bool paused_for_shutdown;
   /* VCPU need affinity restored */
-    bool affinity_broken;
+    uint8_t  affinity_broken;
+#define VCPU_AFFINITY_OVERRIDE    0x01
+#define VCPU_AFFINITY_NMI 0x02


VCPU_AFFINITY_NMI_MCE ?  It is used for more than just NMIs.


Okay.

BTW: The MCE case is never triggered (there is no caller of
pv_raise_interrupt() with TRAP_machine_check). Is this code left in
place on purpose or could it be removed?


It come from the restore_all_guest path in assembly, via process_mce.


Are you sure? I can see that it would call
set_guest_machinecheck_trapbounce(), but I don't see how NMI_MCE_SOFTIRQ
would be triggered on this path, which would be required for hitting
nmi_mce_softirq().


Juergen

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

Re: [Xen-devel] [PATCH 2/2] xen: merge temporary vcpu pinning scenarios

[Jan Beulich]

On 23.07.2019 16:03, Jan Beulich wrote:
> On 23.07.2019 15:44, Juergen Gross wrote:
>> On 23.07.19 14:42, Jan Beulich wrote:
>>> v->processor gets latched into st->processor before raising the softirq,
>>> but can't the vCPU be moved elsewhere by the time the softirq handler
>>> actually gains control? If that's not possible (and if it's not obvious
>>> why, and as you can see it's not obvious to me), then I think a code
>>> comment wants to be added there.
>>
>> You are right, it might be possible for the vcpu to move around.
>>
>> OTOH is it really important to run the target vcpu exactly on the cpu
>> it is executing (or has last executed) at the time the NMI/MCE is being
>> queued? This is in no way related to the cpu the MCE or NMI has been
>> happening on. It is just a random cpu, and so it would be if we'd do the
>> cpu selection when the softirq handler is running.
>>
>> One question to understand the idea nehind all that: _why_ is the vcpu
>> pinned until it does an iret? I could understand if it would be pinned
>> to the cpu where the NMI/MCE was happening, but this is not the case.
> 
> Then it was never finished or got broken, I would guess.

Oh, no. The #MC side use has gone away in 3a91769d6e, without cleaning
up other code. So there doesn't seem to be any such requirement anymore.

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

Re: [Xen-devel] [PATCH 2/2] xen: merge temporary vcpu pinning scenarios

[Juergen Gross]

On 23.07.19 16:14, Jan Beulich wrote:

On 23.07.2019 16:03, Jan Beulich wrote:

On 23.07.2019 15:44, Juergen Gross wrote:

On 23.07.19 14:42, Jan Beulich wrote:

v->processor gets latched into st->processor before raising the softirq,
but can't the vCPU be moved elsewhere by the time the softirq handler
actually gains control? If that's not possible (and if it's not obvious
why, and as you can see it's not obvious to me), then I think a code
comment wants to be added there.


You are right, it might be possible for the vcpu to move around.

OTOH is it really important to run the target vcpu exactly on the cpu
it is executing (or has last executed) at the time the NMI/MCE is being
queued? This is in no way related to the cpu the MCE or NMI has been
happening on. It is just a random cpu, and so it would be if we'd do the
cpu selection when the softirq handler is running.

One question to understand the idea nehind all that: _why_ is the vcpu
pinned until it does an iret? I could understand if it would be pinned
to the cpu where the NMI/MCE was happening, but this is not the case.


Then it was never finished or got broken, I would guess.


Oh, no. The #MC side use has gone away in 3a91769d6e, without cleaning
up other code. So there doesn't seem to be any such requirement anymore.


Ah, okay, so no need any longer to rename VCPU_AFFINITY_NMI. :-)

I'll add a patch removing the MCE cruft.


Juergen


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

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 
133580
 test-amd64-i386-freebsd10-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-libvirt   7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 7 xen-boot fail REGR. vs. 
133580
 test-amd64-i386-qemut-rhel6hvm-intel  7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-qemuu-rhel6hvm-intel  7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-libvirt-pair 10 xen-boot/src_hostfail REGR. vs. 133580
 test-amd64-i386-libvirt-pair 11 xen-boot/dst_hostfail REGR. vs. 133580
 test-amd64-i386-examine   8 reboot   fail REGR. vs. 133580
 test-amd64-i386-xl-raw7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-freebsd10-i386  7 xen-boot   fail REGR. vs. 133580
 test-amd64-i386-xl-qemut-win7-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-qemut-rhel6hvm-amd  7 xen-boot   fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-debianhvm-amd64  7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-pvshim 7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemut-debianhvm-amd64  7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-qemuu-rhel6hvm-amd  7 xen-boot   fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-ws16-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-xl-xsm7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-win10-i386  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-xl-qemut-ws16-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-win7-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-pair 10 xen-boot/src_hostfail REGR. vs. 133580
 test-amd64-i386-pair 11 xen-boot/dst_hostfail REGR. vs. 133580
 test-amd64-i386-xl-shadow 7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-boot fail REGR. vs. 
133580
 test-amd64-i386-xl-qemut-win10-i386  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 
133580
 test-amd64-i386-libvirt-xsm   7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-ovmf-amd64  7 xen-boot  fail REGR. vs. 133580
 test-arm64-arm64-examine11 examine-serial/bootloader fail REGR. vs. 133580
 build-armhf-pvops 6 kernel-build fail REGR. vs. 133580

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-examine  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-arndale   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-cubietruck  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-credit1   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-vhd   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-credit2   1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-rtds  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm  7 xen-boot fail baseline untested
 test-amd64-i386-xl-qemut-debianhvm-i386-xsm  7 xen-boot fail baseline untested
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 133580
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 133580
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 133580
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 133580
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-arm64-arm64-xl-seattle  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail 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-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saveresto

Re: [Xen-devel] [PATCH 2/2] xen: merge temporary vcpu pinning scenarios

[Andrew Cooper]

On 23/07/2019 15:14, Juergen Gross wrote:
> On 23.07.19 15:31, Andrew Cooper wrote:
>> On 23/07/2019 14:25, Juergen Gross wrote:
>>> On 23.07.19 14:26, Andrew Cooper wrote:
 On 23/07/2019 10:20, Juergen Gross wrote:

>    }
>      /*
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index b40c8fd138..721c429454 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -200,7 +200,10 @@ struct vcpu
>    /* VCPU is paused following shutdown request
> (d->is_shutting_down)? */
>    bool paused_for_shutdown;
>    /* VCPU need affinity restored */
> -    bool affinity_broken;
> +    uint8_t  affinity_broken;
> +#define VCPU_AFFINITY_OVERRIDE    0x01
> +#define VCPU_AFFINITY_NMI 0x02

 VCPU_AFFINITY_NMI_MCE ?  It is used for more than just NMIs.
>>>
>>> Okay.
>>>
>>> BTW: The MCE case is never triggered (there is no caller of
>>> pv_raise_interrupt() with TRAP_machine_check). Is this code left in
>>> place on purpose or could it be removed?
>>
>> It come from the restore_all_guest path in assembly, via process_mce.
>
> Are you sure? I can see that it would call
> set_guest_machinecheck_trapbounce(), but I don't see how NMI_MCE_SOFTIRQ
> would be triggered on this path, which would be required for hitting
> nmi_mce_softirq().

Looking at it, I'm not so sure...

There is certainly room for further clean-up here, for anyone with
enough time to disentangle this rats nest.

~Andrew

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

Re: [Xen-devel] [PATCH 2/2] xen: merge temporary vcpu pinning scenarios

[Juergen Gross]

On 23.07.19 16:14, Jan Beulich wrote:

On 23.07.2019 16:03, Jan Beulich wrote:

On 23.07.2019 15:44, Juergen Gross wrote:

On 23.07.19 14:42, Jan Beulich wrote:

v->processor gets latched into st->processor before raising the softirq,
but can't the vCPU be moved elsewhere by the time the softirq handler
actually gains control? If that's not possible (and if it's not obvious
why, and as you can see it's not obvious to me), then I think a code
comment wants to be added there.


You are right, it might be possible for the vcpu to move around.

OTOH is it really important to run the target vcpu exactly on the cpu
it is executing (or has last executed) at the time the NMI/MCE is being
queued? This is in no way related to the cpu the MCE or NMI has been
happening on. It is just a random cpu, and so it would be if we'd do the
cpu selection when the softirq handler is running.

One question to understand the idea nehind all that: _why_ is the vcpu
pinned until it does an iret? I could understand if it would be pinned
to the cpu where the NMI/MCE was happening, but this is not the case.


Then it was never finished or got broken, I would guess.


Oh, no. The #MC side use has gone away in 3a91769d6e, without cleaning
up other code. So there doesn't seem to be any such requirement anymore.


So just to be sure: you are fine for me removing the pinning for NMIs?


Juergen

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

Re: [Xen-devel] [PATCH 2/2] xen: merge temporary vcpu pinning scenarios

[Juergen Gross]

On 23.07.19 15:07, Dario Faggioli wrote:

On Tue, 2019-07-23 at 11:20 +0200, Juergen Gross wrote:

Today there are three scenarios which are pinning vcpus temporarily
to
a single physical cpu:

- NMI/MCE injection into PV domains
- wait_event() handling
- vcpu_pin_override() handling

Each of those cases are handled independently today using their own
temporary cpumask to save the old affinity settings.

The three cases can be combined as the two latter cases will only pin
a vcpu to the physical cpu it is already running on, while
vcpu_pin_override() is allowed to fail.

So merge the three temporary pinning scenarios by only using one
cpumask and a per-vcpu bitmask for specifying which of the three
scenarios is currently active (they are allowed to nest).


Right. And, after this patch, all the three cases above and
suspend/resume, all use cpu_hard_affinity_saved.

Can you add a paragraph, either here in the changelog, on in a comment
(e.g., at the top or inside of vcpu_set_tmp_affinity()), about why this
is all fine?


Yes.


Juergen


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

Re: [Xen-devel] [PATCH 2/2] xen: merge temporary vcpu pinning scenarios

[Jan Beulich]

On 23.07.2019 16:29, Juergen Gross wrote:
> On 23.07.19 16:14, Jan Beulich wrote:
>> On 23.07.2019 16:03, Jan Beulich wrote:
>>> On 23.07.2019 15:44, Juergen Gross wrote:
 On 23.07.19 14:42, Jan Beulich wrote:
> v->processor gets latched into st->processor before raising the softirq,
> but can't the vCPU be moved elsewhere by the time the softirq handler
> actually gains control? If that's not possible (and if it's not obvious
> why, and as you can see it's not obvious to me), then I think a code
> comment wants to be added there.

 You are right, it might be possible for the vcpu to move around.

 OTOH is it really important to run the target vcpu exactly on the cpu
 it is executing (or has last executed) at the time the NMI/MCE is being
 queued? This is in no way related to the cpu the MCE or NMI has been
 happening on. It is just a random cpu, and so it would be if we'd do the
 cpu selection when the softirq handler is running.

 One question to understand the idea nehind all that: _why_ is the vcpu
 pinned until it does an iret? I could understand if it would be pinned
 to the cpu where the NMI/MCE was happening, but this is not the case.
>>>
>>> Then it was never finished or got broken, I would guess.
>>
>> Oh, no. The #MC side use has gone away in 3a91769d6e, without cleaning
>> up other code. So there doesn't seem to be any such requirement anymore.
> 
> So just to be sure: you are fine for me removing the pinning for NMIs?

No, not the pinning as a whole. The forced CPU0 affinity should still
remain. It's just that there's no correlation anymore between the CPU
a vCPU was running on and the CPU it is to be pinned to (temporarily).

What can go away if the #MC part of the logic.

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

Re: [Xen-devel] [PATCH 2/2] xen: merge temporary vcpu pinning scenarios

[Juergen Gross]

On 23.07.19 17:04, Jan Beulich wrote:

On 23.07.2019 16:29, Juergen Gross wrote:

On 23.07.19 16:14, Jan Beulich wrote:

On 23.07.2019 16:03, Jan Beulich wrote:

On 23.07.2019 15:44, Juergen Gross wrote:

On 23.07.19 14:42, Jan Beulich wrote:

v->processor gets latched into st->processor before raising the softirq,
but can't the vCPU be moved elsewhere by the time the softirq handler
actually gains control? If that's not possible (and if it's not obvious
why, and as you can see it's not obvious to me), then I think a code
comment wants to be added there.


You are right, it might be possible for the vcpu to move around.

OTOH is it really important to run the target vcpu exactly on the cpu
it is executing (or has last executed) at the time the NMI/MCE is being
queued? This is in no way related to the cpu the MCE or NMI has been
happening on. It is just a random cpu, and so it would be if we'd do the
cpu selection when the softirq handler is running.

One question to understand the idea nehind all that: _why_ is the vcpu
pinned until it does an iret? I could understand if it would be pinned
to the cpu where the NMI/MCE was happening, but this is not the case.


Then it was never finished or got broken, I would guess.


Oh, no. The #MC side use has gone away in 3a91769d6e, without cleaning
up other code. So there doesn't seem to be any such requirement anymore.


So just to be sure: you are fine for me removing the pinning for NMIs?


No, not the pinning as a whole. The forced CPU0 affinity should still
remain. It's just that there's no correlation anymore between the CPU
a vCPU was running on and the CPU it is to be pinned to (temporarily).


I don't get it. Today vcpu0 of the hardware domain is pinned to the cpu
it was last running on when the NMI happened. Why is that important?
Or do you want to change the logic and pin vcpu0 for NMI handling always
to CPU0?


What can go away if the #MC part of the logic.


Okay.


Juergen


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

Re: [Xen-devel] [PATCH 2/2] xen: merge temporary vcpu pinning scenarios

[Andrew Cooper]

On 23/07/2019 16:22, Juergen Gross wrote:
> On 23.07.19 17:04, Jan Beulich wrote:
>> On 23.07.2019 16:29, Juergen Gross wrote:
>>> On 23.07.19 16:14, Jan Beulich wrote:
 On 23.07.2019 16:03, Jan Beulich wrote:
> On 23.07.2019 15:44, Juergen Gross wrote:
>> On 23.07.19 14:42, Jan Beulich wrote:
>>> v->processor gets latched into st->processor before raising the
>>> softirq,
>>> but can't the vCPU be moved elsewhere by the time the softirq
>>> handler
>>> actually gains control? If that's not possible (and if it's not
>>> obvious
>>> why, and as you can see it's not obvious to me), then I think a
>>> code
>>> comment wants to be added there.
>>
>> You are right, it might be possible for the vcpu to move around.
>>
>> OTOH is it really important to run the target vcpu exactly on the
>> cpu
>> it is executing (or has last executed) at the time the NMI/MCE is
>> being
>> queued? This is in no way related to the cpu the MCE or NMI has been
>> happening on. It is just a random cpu, and so it would be if we'd
>> do the
>> cpu selection when the softirq handler is running.
>>
>> One question to understand the idea nehind all that: _why_ is the
>> vcpu
>> pinned until it does an iret? I could understand if it would be
>> pinned
>> to the cpu where the NMI/MCE was happening, but this is not the
>> case.
>
> Then it was never finished or got broken, I would guess.

 Oh, no. The #MC side use has gone away in 3a91769d6e, without cleaning
 up other code. So there doesn't seem to be any such requirement
 anymore.
>>>
>>> So just to be sure: you are fine for me removing the pinning for NMIs?
>>
>> No, not the pinning as a whole. The forced CPU0 affinity should still
>> remain. It's just that there's no correlation anymore between the CPU
>> a vCPU was running on and the CPU it is to be pinned to (temporarily).
>
> I don't get it. Today vcpu0 of the hardware domain is pinned to the cpu
> it was last running on when the NMI happened. Why is that important?
> Or do you want to change the logic and pin vcpu0 for NMI handling always
> to CPU0?

Its (allegedly) for when dom0 knows some system-specific way of getting
extra information out of the platform, that happens to be core-specific.

There are rare cases where SMI's need to be executed on CPU0, and I
wouldn't put it past hardware designers to have similar aspects for NMIs.

That said, as soon as the gaping security hole which is the
default-readibility of all MSRs, I bet the utility of this pinning
mechanism will be 0.

~Andrew

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

Re: [Xen-devel] [PATCH] docs/sphinx: todo/wishlist

[Juergen Gross]

On 23.07.19 15:38, Andrew Cooper wrote:

On 23/07/2019 05:36, Juergen Gross wrote:

On 22.07.19 21:20, Andrew Cooper wrote:

diff --git a/docs/misc/wishlist.rst b/docs/misc/wishlist.rst
new file mode 100644
index 00..6cdb47d6e7
--- /dev/null
+++ b/docs/misc/wishlist.rst
@@ -0,0 +1,53 @@
+Development Wishlist
+
+
+Remove xenstored's dependencies on unstable interfaces
+--
+
+Various xenstored implementations use libxc for two purposes.  It
would be a
+substantial advantage to move xenstored onto entirely stable
interfaces, which
+disconnects it from the internal of the libxc.
+
+1. Foreign mapping of the store ring
+
+
+This is obsolete since :xen-cs:`6a2de353a9` (2012) which allocated
grant
+entries instead, to allow xenstored to function as a stub-domain
without dom0
+permissions.  :xen-cs:`38eeb3864d` dropped foreign mapping for
cxenstored.
+However, there are no OCaml bindings for libxengnttab.
+
+Work Items:
+
+* Minimal ``tools/ocaml/libs/xg/`` binding for ``tools/libs/gnttab/``.
+* Replicate :xen-cs:`38eeb3864d` for oxenstored as well.
+
+
+2. Figuring out which domain(s) have gone away
+~~
+
+Currently, the handling of domains is asymmetric.
+
+* When a domain is created, the toolstack explicitly sends an
+  ``XS_INTRODUCE(domid, store mfn, store evtchn)`` message to
xenstored, to
+  cause xenstored to connect to the guest ring, and fire the
+  ``@introduceDomain`` watch.
+* When a domain is destroyed, Xen fires ``VIRQ_DOM_EXC`` which is
bound by
+  xenstored, rather than the toolstack.  xenstored updates its idea
of the
+  status of domains, and fires the ``@releaseDomain`` watch.
+
+Xenstored uses ``xc_domain_getinfo()``, to work out which domain(s)
have gone
+away, and only cares about the shutdown status.
+
+Furthermore, ``@releaseDomain`` (like ``VIRQ_DOM_EXC``) is a single-bit
+message, which requires all listeners to evaluate whether the
message applies
+to them or not.  This results in a flurry of ``xc_domain_getinfo()``
calls
+from multiple entities in the system, which all serialise on the
domctl lock
+in Xen.
+
+Work Items:
+
+* Figure out how shutdown status can be expressed in a stable way
from Xen.
+* Figure out if ``VIRQ_DOM_EXC`` and ``@releaseDomain`` can be
extended to
+  carry at least a domid, to make domain shutdown scale better.


@releaseDomain (and @introduceDomain) can't be extended, we'd need to
add another watch path like @domainStatus//. Xenstored
could advertise its capability to raise this watch in /tool/xenstored.


I guess I was being a bit fast and loose with terminology.  I didn't
intend to imply "literally modify @{introduce,release}Domain", as they
are already fixed ABIs, but more to "compatibly build something which is
better".


Okay.



That scheme would work for improved @releaseDomain, but it wouldn't work
for an improved introduce.  Introduce needs a single key to watch on,
which hands back the domid so you don't need to go searching for it.


Yes, and? Its perfectly fine to set a watch firing if anything below
@domainStatus is changing.





As VIRQ_DOM_EXC is just an event I don't see how the domid could be
passed by it. I guess we'd need e.g. a shared memory area which the
domain registered for VIRQ_DOM_EXC could map and which would contain a
bitmap (one bit per domain). The hypervisor would set the bit on a
status change and fire VIRQ_DOM_EXC, xenstored would look for a set
bit, clear it and read the status of the related domain.


The point here is to avoid using xc_domain_getinfo() in the first place,
so there needs to be no "getting the status of the domain".


I'd guess a single xc_domain_getinfo() in the tools wouldn't be so
problematic. The caller would know the domid already, so no need to
query all domains.


DOM_EXC is fired for domain_shutdown() only (but for reasons which
escape me, fired twice).  Given that a domid is a 15 bit number, a
bitmap of all domains does fit within a single 4k page.


Firing twice is needed: first time for disconnecting all backends
and the second time for cleaning up when the domain is completely
gone.



All xenstored needs to know is that the domain shut down - nothing
else.  Given a mapping such as the above, xenstored, on getting
VIRQ_DOM_EXC would memcpy the page sideways, then loop over the page
looking for bits which have become set, notify for each of those
domains, and clear the bit in the page.


Right, that was the idea.






+* Figure out if ``VIRQ_DOM_EXC`` would better be bound by the
toolstack,
+  rather than xenstored.


No, I don't think so. This would need another daemon.


There is exactly one toolstack which is nominally daemonless, and it
doesn't account for any substantial proportion of the Xen install-base,
worldwide.

The fact xl might not cope has no relevance to the technical discussion
of whether the current asymmetric setup is sa

[Xen-devel] [PATCH 1/2] x86/iommu: avoid mapping the interrupt address range for hwdom

[Roger Pau Monne]

Current code only prevent mapping the lapic page into the guest
physical memory map. Expand the range to be 0xFEEx_ as described
in the Intel VTd specification section 3.13 "Handling Requests to
Interrupt Address Range".

AMD also lists this address range in the AMD SR5690 Databook, section
2.4.4 "MSI Interrupt Handling and MSI to HT Interrupt Conversion".

Requested-by: Andrew Cooper 
Signed-off-by: Roger Pau Monné 
---
Cc: Jan Beulich 
Cc: Andrew Cooper 
---
 xen/drivers/passthrough/x86/iommu.c | 16 +++-
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/xen/drivers/passthrough/x86/iommu.c 
b/xen/drivers/passthrough/x86/iommu.c
index 0fa6dcc3fd..88a87cf7a4 100644
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -226,19 +226,9 @@ static bool __hwdom_init hwdom_iommu_map(const struct 
domain *d,
 return false;
 }
 
-/*
- * Check that it doesn't overlap with the LAPIC
- * TODO: if the guest relocates the MMIO area of the LAPIC Xen should make
- * sure there's nothing in the new address that would prevent trapping.
- */
-if ( has_vlapic(d) )
-{
-const struct vcpu *v;
-
-for_each_vcpu(d, v)
-if ( pfn == PFN_DOWN(vlapic_base_address(vcpu_vlapic(v))) )
-return false;
-}
+/* Check that it doesn't overlap with the Interrupt Address Range. */
+if ( pfn >= 0xfee00 && pfn <= 0xfeeff )
+return false;
 /* ... or the IO-APIC */
 for ( i = 0; has_vioapic(d) && i < d->arch.hvm.nr_vioapics; i++ )
 if ( pfn == PFN_DOWN(domain_vioapic(d, i)->base_address) )
-- 
2.20.1 (Apple Git-117)


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

[Xen-devel] [PATCH 2/2] x86/iommu: avoid mapping the APIC configuration space for hwdom

[Roger Pau Monne]

Current code only prevents mapping the io-apic page into the guest
physical memory map. Expand the range to be 0xFECx_ as described
in the Intel 3 Series Chipset Datasheet section 3.3.1 "APIC
Configuration Space (FEC0_h–FECF_h)".

AMD also lists this address range in the AMD SR5690 Databook, section
2.4.2 "Non-SB IOAPIC Support".

Requested-by: Andrew Cooper 
Signed-off-by: Roger Pau Monné 
---
Cc: Jan Beulich 
Cc: Andrew Cooper 
---
 xen/drivers/passthrough/x86/iommu.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/xen/drivers/passthrough/x86/iommu.c 
b/xen/drivers/passthrough/x86/iommu.c
index 88a87cf7a4..4dabfb2391 100644
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -195,7 +195,7 @@ static bool __hwdom_init hwdom_iommu_map(const struct 
domain *d,
  unsigned long max_pfn)
 {
 mfn_t mfn = _mfn(pfn);
-unsigned int i, type;
+unsigned int type;
 
 /*
  * Set up 1:1 mapping for dom0. Default to include only conventional RAM
@@ -229,10 +229,9 @@ static bool __hwdom_init hwdom_iommu_map(const struct 
domain *d,
 /* Check that it doesn't overlap with the Interrupt Address Range. */
 if ( pfn >= 0xfee00 && pfn <= 0xfeeff )
 return false;
-/* ... or the IO-APIC */
-for ( i = 0; has_vioapic(d) && i < d->arch.hvm.nr_vioapics; i++ )
-if ( pfn == PFN_DOWN(domain_vioapic(d, i)->base_address) )
-return false;
+/* ... or the APIC Configuration Space. */
+if ( pfn >= 0xfec00 && pfn <= 0xfecff )
+return false;
 /*
  * ... or the PCIe MCFG regions.
  * TODO: runtime added MMCFG regions are not checked to make sure they
-- 
2.20.1 (Apple Git-117)


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

[Xen-devel] [PATCH 0/2] x86/iommu: fixes to hwdom_iommu_map

[Roger Pau Monne]

Hello,

The following series contains two small fixes to hwdom_iommu_map in
order to expand the forbidden ranges to cover the full Interrupt Address
Range and the APIC Configuration Space.

Thanks, Roger.

Roger Pau Monne (2):
  x86/iommu: avoid mapping the interrupt address range for hwdom
  x86/iommu: avoid mapping the APIC configuration space for hwdom

 xen/drivers/passthrough/x86/iommu.c | 25 +++--
 1 file changed, 7 insertions(+), 18 deletions(-)

-- 
2.20.1 (Apple Git-117)


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

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

[osstest service owner]

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

Failures :-/ but no regressions.

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

version targeted for testing:
 xen  0d10a77cc98dda1b8f9a96954149a872f46048c9
baseline version:
 xen  f56813f3470c5b4987963c3c41e4fe16b95c5a3f

Last test of basis   139261  2019-07-22 18:00:31 Z0 days
Testing same since   139282  2019-07-23 13:00:45 Z0 days1 attempts


People who touched revisions under test:
  Juergen Gross 
  Viktor Mitin 
  Viktor Mitin 

jobs:
 build-arm64-xsm  pass
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-arm64-arm64-xl-xsm  pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-amd64-libvirt pass



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

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

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

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


Pushing revision :

To xenbits.xen.org:/home/xen/git/xen.git
   f56813f347..0d10a77cc9  0d10a77cc98dda1b8f9a96954149a872f46048c9 -> smoke

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

Re: [Xen-devel] [PATCH 2/2] xen: merge temporary vcpu pinning scenarios

[Juergen Gross]

On 23.07.19 17:29, Andrew Cooper wrote:

On 23/07/2019 16:22, Juergen Gross wrote:

On 23.07.19 17:04, Jan Beulich wrote:

On 23.07.2019 16:29, Juergen Gross wrote:

On 23.07.19 16:14, Jan Beulich wrote:

On 23.07.2019 16:03, Jan Beulich wrote:

On 23.07.2019 15:44, Juergen Gross wrote:

On 23.07.19 14:42, Jan Beulich wrote:

v->processor gets latched into st->processor before raising the
softirq,
but can't the vCPU be moved elsewhere by the time the softirq
handler
actually gains control? If that's not possible (and if it's not
obvious
why, and as you can see it's not obvious to me), then I think a
code
comment wants to be added there.


You are right, it might be possible for the vcpu to move around.

OTOH is it really important to run the target vcpu exactly on the
cpu
it is executing (or has last executed) at the time the NMI/MCE is
being
queued? This is in no way related to the cpu the MCE or NMI has been
happening on. It is just a random cpu, and so it would be if we'd
do the
cpu selection when the softirq handler is running.

One question to understand the idea nehind all that: _why_ is the
vcpu
pinned until it does an iret? I could understand if it would be
pinned
to the cpu where the NMI/MCE was happening, but this is not the
case.


Then it was never finished or got broken, I would guess.


Oh, no. The #MC side use has gone away in 3a91769d6e, without cleaning
up other code. So there doesn't seem to be any such requirement
anymore.


So just to be sure: you are fine for me removing the pinning for NMIs?


No, not the pinning as a whole. The forced CPU0 affinity should still
remain. It's just that there's no correlation anymore between the CPU
a vCPU was running on and the CPU it is to be pinned to (temporarily).


I don't get it. Today vcpu0 of the hardware domain is pinned to the cpu
it was last running on when the NMI happened. Why is that important?
Or do you want to change the logic and pin vcpu0 for NMI handling always
to CPU0?


Its (allegedly) for when dom0 knows some system-specific way of getting
extra information out of the platform, that happens to be core-specific.

There are rare cases where SMI's need to be executed on CPU0, and I
wouldn't put it past hardware designers to have similar aspects for NMIs.


Understood. But today vcpu0 is _not_ bound to CPU0, but to any cpu it
happened to run on.



That said, as soon as the gaping security hole which is the
default-readibility of all MSRs, I bet the utility of this pinning
mechanism will be 0.


And my reasoning is that this is the case today already, as there is
no pinning to CPU0 done, at least not on purpose.


Juergen


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

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

[osstest service owner]

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

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf bb824f685d760f560bb3c3fb14af394ab3b3544f
baseline version:
 ovmf 5f89bcc4604ea9e439039d873e34a8c06b47c707

Last test of basis   139241  2019-07-22 01:40:01 Z1 days
Testing same since   139262  2019-07-22 18:15:17 Z0 days1 attempts


People who touched revisions under test:
  Bob Feng 
  Feng, Bob C 

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



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

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

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

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


Pushing revision :

To xenbits.xen.org:/home/xen/git/osstest/ovmf.git
   5f89bcc460..bb824f685d  bb824f685d760f560bb3c3fb14af394ab3b3544f -> 
xen-tested-master

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

Re: [Xen-devel] [PATCH 2/2] xen: merge temporary vcpu pinning scenarios

[Andrew Cooper]

On 23/07/2019 16:53, Juergen Gross wrote:
> On 23.07.19 17:29, Andrew Cooper wrote:
>> On 23/07/2019 16:22, Juergen Gross wrote:
>>> On 23.07.19 17:04, Jan Beulich wrote:
 On 23.07.2019 16:29, Juergen Gross wrote:
> On 23.07.19 16:14, Jan Beulich wrote:
>> On 23.07.2019 16:03, Jan Beulich wrote:
>>> On 23.07.2019 15:44, Juergen Gross wrote:
 On 23.07.19 14:42, Jan Beulich wrote:
> v->processor gets latched into st->processor before raising the
> softirq,
> but can't the vCPU be moved elsewhere by the time the softirq
> handler
> actually gains control? If that's not possible (and if it's not
> obvious
> why, and as you can see it's not obvious to me), then I think a
> code
> comment wants to be added there.

 You are right, it might be possible for the vcpu to move around.

 OTOH is it really important to run the target vcpu exactly on the
 cpu
 it is executing (or has last executed) at the time the NMI/MCE is
 being
 queued? This is in no way related to the cpu the MCE or NMI has
 been
 happening on. It is just a random cpu, and so it would be if we'd
 do the
 cpu selection when the softirq handler is running.

 One question to understand the idea nehind all that: _why_ is the
 vcpu
 pinned until it does an iret? I could understand if it would be
 pinned
 to the cpu where the NMI/MCE was happening, but this is not the
 case.
>>>
>>> Then it was never finished or got broken, I would guess.
>>
>> Oh, no. The #MC side use has gone away in 3a91769d6e, without
>> cleaning
>> up other code. So there doesn't seem to be any such requirement
>> anymore.
>
> So just to be sure: you are fine for me removing the pinning for
> NMIs?

 No, not the pinning as a whole. The forced CPU0 affinity should still
 remain. It's just that there's no correlation anymore between the CPU
 a vCPU was running on and the CPU it is to be pinned to (temporarily).
>>>
>>> I don't get it. Today vcpu0 of the hardware domain is pinned to the cpu
>>> it was last running on when the NMI happened. Why is that important?
>>> Or do you want to change the logic and pin vcpu0 for NMI handling
>>> always
>>> to CPU0?
>>
>> Its (allegedly) for when dom0 knows some system-specific way of getting
>> extra information out of the platform, that happens to be core-specific.
>>
>> There are rare cases where SMI's need to be executed on CPU0, and I
>> wouldn't put it past hardware designers to have similar aspects for
>> NMIs.
>
> Understood. But today vcpu0 is _not_ bound to CPU0, but to any cpu it
> happened to run on.
>
>>
>> That said, as soon as the gaping security hole which is the
>> default-readibility of all MSRs, I bet the utility of this pinning
>> mechanism will be 0.
>
> And my reasoning is that this is the case today already, as there is
> no pinning to CPU0 done, at least not on purpose.

Based on this analysis, I'd be tempted to drop the pinning completely. 
It clearly isn't working in a rational way.

~Andrew

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

Re: [Xen-devel] [PATCH 2/2] xen: merge temporary vcpu pinning scenarios

[Jan Beulich]

On 23.07.2019 17:22, Juergen Gross wrote:
> On 23.07.19 17:04, Jan Beulich wrote:
>> On 23.07.2019 16:29, Juergen Gross wrote:
>>> On 23.07.19 16:14, Jan Beulich wrote:
 Oh, no. The #MC side use has gone away in 3a91769d6e, without cleaning
 up other code. So there doesn't seem to be any such requirement anymore.
>>>
>>> So just to be sure: you are fine for me removing the pinning for NMIs?
>>
>> No, not the pinning as a whole. The forced CPU0 affinity should still
>> remain. It's just that there's no correlation anymore between the CPU
>> a vCPU was running on and the CPU it is to be pinned to (temporarily).
> 
> I don't get it. Today vcpu0 of the hardware domain is pinned to the cpu
> it was last running on when the NMI happened. Why is that important?
> Or do you want to change the logic and pin vcpu0 for NMI handling always
> to CPU0?

Hmm, I've been confused. To me it doesn't make sense to force
affinity other than to CPU0 for an NMI. Hence I was mis-reading
the original send_guest_trap() invocation (in particular its
middle argument). Looking at commit 355b0469a8 ("x86: MCA support")
I would guess the affinity change for NMI delivery was done by
mistake, or at best just to not distinguish NMI from #MC.

As a result, considering that before that there was no affinity
change at all, and considering further than changing to other
than CPU0 doesn't really make sense, but we got away with not
doing so for so many years, I think I agree that it can be ripped
out altogether. Personally I'd prefer a fix to make it use CPU0,
but I'm in no way going to insist.

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

[Xen-devel] [PATCH 4/6] x86/domain: remove the 'oos_off' flag

[Paul Durrant]

The flag is not needed since the domain 'createflags' can now be tested
directly.

Signed-off-by: Paul Durrant 
---
Cc: Tim Deegan 
Cc: George Dunlap 
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Wei Liu 
Cc: "Roger Pau Monné" 
---
 xen/arch/x86/mm/shadow/common.c | 3 +--
 xen/include/asm-x86/domain.h| 1 -
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c
index 320ea0db21..2c7fafa4fb 100644
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -62,7 +62,6 @@ int shadow_domain_init(struct domain *d)
 
 #if (SHADOW_OPTIMIZATIONS & SHOPT_OUT_OF_SYNC)
 d->arch.paging.shadow.oos_active = 0;
-d->arch.paging.shadow.oos_off = d->createflags & XEN_DOMCTL_CDF_oos_off;
 #endif
 d->arch.paging.shadow.pagetable_dying_op = 0;
 
@@ -2523,7 +2522,7 @@ static void sh_update_paging_modes(struct vcpu *v)
 #if (SHADOW_OPTIMIZATIONS & SHOPT_OUT_OF_SYNC)
 /* We need to check that all the vcpus have paging enabled to
  * unsync PTs. */
-if ( is_hvm_domain(d) && !d->arch.paging.shadow.oos_off )
+if ( is_hvm_domain(d) && !(d->createflags & XEN_DOMCTL_CDF_oos_off) )
 {
 int pe = 1;
 struct vcpu *vptr;
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 933b85901f..5f9899469c 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -115,7 +115,6 @@ struct shadow_domain {
 
 /* OOS */
 bool_t oos_active;
-bool_t oos_off;
 
 /* Has this domain ever used HVMOP_pagetable_dying? */
 bool_t pagetable_dying_op;
-- 
2.20.1.2.gb21ebb671


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

[Xen-devel] [PATCH 2/6] domain: remove 'guest_type' field (and enum guest_type)

[Paul Durrant]

The enum guest_type was introduced in commit 6c6492780ea "pvh prep:
introduce pv guest type and has_hvm_container macros" to allow a new guest
type, distinct from either PV or HVM guest types, to be added in commit
8271d6522c6 "pvh: introduce PVH guest type". Subsequently, commit
33e5c32559e "x86: remove PVHv1 code" removed this third guest type.

This patch removes the struct domain field and enumeration as the guest
type can now be trivially determined from the 'createflags' field.

Signed-off-by: Paul Durrant 
---
Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Julien Grall 
Cc: Konrad Rzeszutek Wilk 
Cc: Stefano Stabellini 
Cc: Tim Deegan 
Cc: Wei Liu 
---
 xen/common/domain.c |  4 
 xen/common/domctl.c | 10 +-
 xen/common/kernel.c |  9 ++---
 xen/include/xen/sched.h | 14 --
 4 files changed, 7 insertions(+), 30 deletions(-)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index d559c8898e..6f405d2541 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -355,10 +355,6 @@ struct domain *domain_create(domid_t domid,
 hardware_domain = d;
 }
 
-/* Sort out our idea of is_{pv,hvm}_domain().  All system domains are PV. 
*/
-d->guest_type = ((d->createflags & XEN_DOMCTL_CDF_hvm_guest)
- ? guest_type_hvm : guest_type_pv);
-
 TRACE_1D(TRC_DOM0_DOM_ADD, d->domain_id);
 
 /*
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 72a44953d0..ef6714c0aa 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -187,17 +187,9 @@ void getdomaininfo(struct domain *d, struct 
xen_domctl_getdomaininfo *info)
 (d->controller_pause_count > 0  ? XEN_DOMINF_paused: 0) |
 (d->debugger_attached   ? XEN_DOMINF_debugged  : 0) |
 (d->is_xenstore ? XEN_DOMINF_xs_domain : 0) |
+(is_hvm_domain(d)   ? XEN_DOMINF_hvm_guest : 0) |
 d->shutdown_code << XEN_DOMINF_shutdownshift;
 
-switch ( d->guest_type )
-{
-case guest_type_hvm:
-info->flags |= XEN_DOMINF_hvm_guest;
-break;
-default:
-break;
-}
-
 xsm_security_domaininfo(d, info);
 
 info->tot_pages = d->tot_pages;
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 612575430f..f7628d73ce 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -474,19 +474,14 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 fi.submap |= (1U << XENFEAT_ARM_SMCCC_supported);
 #endif
 #ifdef CONFIG_X86
-switch ( d->guest_type )
-{
-case guest_type_pv:
+if ( is_pv_domain(d) )
 fi.submap |= (1U << XENFEAT_mmu_pt_update_preserve_ad) |
  (1U << XENFEAT_highmem_assist) |
  (1U << XENFEAT_gnttab_map_avail_bits);
-break;
-case guest_type_hvm:
+else
 fi.submap |= (1U << XENFEAT_hvm_safe_pvclock) |
  (1U << XENFEAT_hvm_callback_vector) |
  (has_pirq(d) ? (1U << XENFEAT_hvm_pirqs) : 0);
-break;
-}
 #endif
 break;
 default:
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index edae372c2b..9a98857237 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -302,10 +302,6 @@ struct vm_event_domain
 
 struct evtchn_port_ops;
 
-enum guest_type {
-guest_type_pv, guest_type_hvm
-};
-
 struct domain
 {
 unsigned int createflags;
@@ -357,8 +353,6 @@ struct domain
 struct radix_tree_root pirq_tree;
 unsigned int nr_pirqs;
 
-enum guest_type guest_type;
-
 /* Is this guest dying (i.e., a zombie)? */
 enum { DOMDYING_alive, DOMDYING_dying, DOMDYING_dead } is_dying;
 
@@ -918,8 +912,8 @@ void watchdog_domain_destroy(struct domain *d);
 
 static inline bool is_pv_domain(const struct domain *d)
 {
-return IS_ENABLED(CONFIG_PV)
-   ? evaluate_nospec(d->guest_type == guest_type_pv) : false;
+return IS_ENABLED(CONFIG_PV) &&
+!evaluate_nospec(d->createflags & XEN_DOMCTL_CDF_hvm_guest);
 }
 
 static inline bool is_pv_vcpu(const struct vcpu *v)
@@ -950,8 +944,8 @@ static inline bool is_pv_64bit_vcpu(const struct vcpu *v)
 #endif
 static inline bool is_hvm_domain(const struct domain *d)
 {
-return IS_ENABLED(CONFIG_HVM)
-   ? evaluate_nospec(d->guest_type == guest_type_hvm) : false;
+return IS_ENABLED(CONFIG_HVM) &&
+evaluate_nospec(d->createflags & XEN_DOMCTL_CDF_hvm_guest);
 }
 
 static inline bool is_hvm_vcpu(const struct vcpu *v)
-- 
2.20.1.2.gb21ebb671


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

[Xen-devel] [PATCH 5/6] domain: remove the 'is_xenstore' flag

[Paul Durrant]

This patch introduces a convenience macro, is_xenstore_domain(), which
tests the domain 'createflags' directly and then uses that in place of
the 'is_xenstore' flag.

Signed-off-by: Paul Durrant 
---
Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Julien Grall 
Cc: Konrad Rzeszutek Wilk 
Cc: Stefano Stabellini 
Cc: Tim Deegan 
Cc: Wei Liu 
Cc: Daniel De Graaf 
---
 xen/common/domain.c | 5 +
 xen/common/domctl.c | 2 +-
 xen/include/xen/sched.h | 7 +--
 xen/include/xsm/dummy.h | 2 +-
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index 6f405d2541..5703da357f 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -429,11 +429,8 @@ struct domain *domain_create(domid_t domid,
 watchdog_domain_init(d);
 init_status |= INIT_watchdog;
 
-if ( d->createflags & XEN_DOMCTL_CDF_xs_domain )
-{
-d->is_xenstore = 1;
+if ( is_xenstore_domain(d) )
 d->disable_migrate = 1;
-}
 
 d->iomem_caps = rangeset_new(d, "I/O Memory", 
RANGESETF_prettyprint_hex);
 d->irq_caps   = rangeset_new(d, "Interrupts", 0);
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index ef6714c0aa..f2b582812c 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -186,7 +186,7 @@ void getdomaininfo(struct domain *d, struct 
xen_domctl_getdomaininfo *info)
 (d->is_shut_down? XEN_DOMINF_shutdown  : 0) |
 (d->controller_pause_count > 0  ? XEN_DOMINF_paused: 0) |
 (d->debugger_attached   ? XEN_DOMINF_debugged  : 0) |
-(d->is_xenstore ? XEN_DOMINF_xs_domain : 0) |
+(is_xenstore_domain(d)  ? XEN_DOMINF_xs_domain : 0) |
 (is_hvm_domain(d)   ? XEN_DOMINF_hvm_guest : 0) |
 d->shutdown_code << XEN_DOMINF_shutdownshift;
 
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 9a98857237..1033ca0e8c 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -374,8 +374,6 @@ struct domain
 bool is_privileged;
 /* Can this guest access the Xen console? */
 bool is_console;
-/* Is this a xenstore domain (not dom0)? */
-bool is_xenstore;
 /* Non-migratable and non-restoreable? */
 bool disable_migrate;
 /* Is this guest being debugged by dom0? */
@@ -972,6 +970,11 @@ static inline bool is_vcpu_online(const struct vcpu *v)
 return !test_bit(_VPF_down, &v->pause_flags);
 }
 
+static inline bool is_xenstore_domain(const struct domain *d)
+{
+return d->createflags & XEN_DOMCTL_CDF_xs_domain;
+}
+
 extern bool sched_smt_power_savings;
 
 extern enum cpufreq_controller {
diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index ef52bb1764..b8e185e6fa 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -79,7 +79,7 @@ static always_inline int xsm_default_action(
 {
 return 0;
 case XSM_XS_PRIV:
-if ( src->is_xenstore )
+if ( is_xenstore_domain(src) )
 return 0;
 }
 /* fall through */
-- 
2.20.1.2.gb21ebb671


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

[Xen-devel] [PATCH 6/6] x86/domain: remove the 's3_integrity' flag

[Paul Durrant]

The flag is not needed since the domain 'createflags' can now be tested
directly.

Signed-off-by: Paul Durrant 
---
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Wei Liu 
Cc: "Roger Pau Monné" 
Cc: Gang Wei 
Cc: Shane Wang 
---
 xen/arch/x86/domain.c| 2 --
 xen/arch/x86/tboot.c | 2 +-
 xen/include/asm-x86/domain.h | 2 --
 3 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 65f47a7627..2203882445 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -538,8 +538,6 @@ int arch_domain_create(struct domain *d,
d->domain_id);
 }
 
-d->arch.s3_integrity = config->flags & XEN_DOMCTL_CDF_s3_integrity;
-
 emflags = config->arch.emulation_flags;
 
 if ( is_hardware_domain(d) && is_pv_domain(d) )
diff --git a/xen/arch/x86/tboot.c b/xen/arch/x86/tboot.c
index f3fdee4d39..1a7e44a8a9 100644
--- a/xen/arch/x86/tboot.c
+++ b/xen/arch/x86/tboot.c
@@ -212,7 +212,7 @@ static void tboot_gen_domain_integrity(const uint8_t 
key[TB_KEY_SIZE],
 vmac_set_key((uint8_t *)key, &ctx);
 for_each_domain( d )
 {
-if ( !d->arch.s3_integrity )
+if ( !(d->createflags & XEN_DOMCTL_CDF_s3_integrity) )
 continue;
 printk("MACing Domain %u\n", d->domain_id);
 
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 5f9899469c..5c038a1065 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -295,8 +295,6 @@ struct arch_domain
 uint32_t pci_cf8;
 uint8_t cmos_idx;
 
-bool_t s3_integrity;
-
 union {
 struct pv_domain pv;
 struct hvm_domain hvm;
-- 
2.20.1.2.gb21ebb671


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

[Xen-devel] [PATCH 1/6] domain: stash xen_domctl_createdomain flags in struct domain

[Paul Durrant]

These are canonical source of data used to set various other flags. If
they are available directly in struct domain then the other flags are no
longer needed.

This patch simply copies the flags into a new 'createflags' field in
struct domain. Subsequent patches will do the related clean-up work.

Signed-off-by: Paul Durrant 
---
Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Julien Grall 
Cc: Konrad Rzeszutek Wilk 
Cc: Stefano Stabellini 
Cc: Tim Deegan 
Cc: Wei Liu 
---
 xen/common/domain.c | 6 --
 xen/include/xen/sched.h | 1 +
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index 55aa759b75..d559c8898e 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -333,6 +333,8 @@ struct domain *domain_create(domid_t domid,
 if ( (d = alloc_domain_struct()) == NULL )
 return ERR_PTR(-ENOMEM);
 
+d->createflags = config ? config->flags : 0;
+
 /* Sort out our idea of is_system_domain(). */
 d->domain_id = domid;
 
@@ -354,7 +356,7 @@ struct domain *domain_create(domid_t domid,
 }
 
 /* Sort out our idea of is_{pv,hvm}_domain().  All system domains are PV. 
*/
-d->guest_type = ((config && (config->flags & XEN_DOMCTL_CDF_hvm_guest))
+d->guest_type = ((d->createflags & XEN_DOMCTL_CDF_hvm_guest)
  ? guest_type_hvm : guest_type_pv);
 
 TRACE_1D(TRC_DOM0_DOM_ADD, d->domain_id);
@@ -431,7 +433,7 @@ struct domain *domain_create(domid_t domid,
 watchdog_domain_init(d);
 init_status |= INIT_watchdog;
 
-if ( config->flags & XEN_DOMCTL_CDF_xs_domain )
+if ( d->createflags & XEN_DOMCTL_CDF_xs_domain )
 {
 d->is_xenstore = 1;
 d->disable_migrate = 1;
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index b40c8fd138..edae372c2b 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -308,6 +308,7 @@ enum guest_type {
 
 struct domain
 {
+unsigned int createflags;
 domid_t  domain_id;
 
 unsigned int max_vcpus;
-- 
2.20.1.2.gb21ebb671


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

[Xen-devel] [PATCH 3/6] x86/hvm/domain: remove the 'hap_enabled' flag

[Paul Durrant]

The hap_enabled() macro can determine whether the feature is available
using the domain 'createflags'; there is no need for a separate flag.

Signed-off-by: Paul Durrant 
---
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Wei Liu 
Cc: "Roger Pau Monné" 
---
 xen/arch/x86/domain.c| 7 +--
 xen/arch/x86/mm/paging.c | 4 ++--
 xen/arch/x86/mm/shadow/common.c  | 4 ++--
 xen/include/asm-x86/hvm/domain.h | 9 +++--
 xen/include/asm-x86/paging.h | 2 +-
 xen/include/asm-x86/shadow.h | 2 +-
 6 files changed, 10 insertions(+), 18 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index ea55160887..65f47a7627 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -564,12 +564,7 @@ int arch_domain_create(struct domain *d,
 HYPERVISOR_COMPAT_VIRT_START(d) =
 is_pv_domain(d) ? __HYPERVISOR_COMPAT_VIRT_START : ~0u;
 
-/* Need to determine if HAP is enabled before initialising paging */
-if ( is_hvm_domain(d) )
-d->arch.hvm.hap_enabled =
-hvm_hap_supported() && (config->flags & XEN_DOMCTL_CDF_hap);
-
-if ( (rc = paging_domain_init(d, config->flags)) != 0 )
+if ( (rc = paging_domain_init(d)) != 0 )
 goto fail;
 paging_initialised = true;
 
diff --git a/xen/arch/x86/mm/paging.c b/xen/arch/x86/mm/paging.c
index 011089368a..69aa228e46 100644
--- a/xen/arch/x86/mm/paging.c
+++ b/xen/arch/x86/mm/paging.c
@@ -632,7 +632,7 @@ void paging_log_dirty_init(struct domain *d, const struct 
log_dirty_ops *ops)
 /*   CODE FOR PAGING SUPPORT*/
 //
 /* Domain paging struct initialization. */
-int paging_domain_init(struct domain *d, unsigned int domcr_flags)
+int paging_domain_init(struct domain *d)
 {
 int rc;
 
@@ -653,7 +653,7 @@ int paging_domain_init(struct domain *d, unsigned int 
domcr_flags)
 if ( hap_enabled(d) )
 hap_domain_init(d);
 else
-rc = shadow_domain_init(d, domcr_flags);
+rc = shadow_domain_init(d);
 
 return rc;
 }
diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c
index fa18de0bb6..320ea0db21 100644
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -46,7 +46,7 @@ static void sh_clean_dirty_bitmap(struct domain *);
 
 /* Set up the shadow-specific parts of a domain struct at start of day.
  * Called for every domain from arch_domain_create() */
-int shadow_domain_init(struct domain *d, unsigned int domcr_flags)
+int shadow_domain_init(struct domain *d)
 {
 static const struct log_dirty_ops sh_ops = {
 .enable  = sh_enable_log_dirty,
@@ -62,7 +62,7 @@ int shadow_domain_init(struct domain *d, unsigned int 
domcr_flags)
 
 #if (SHADOW_OPTIMIZATIONS & SHOPT_OUT_OF_SYNC)
 d->arch.paging.shadow.oos_active = 0;
-d->arch.paging.shadow.oos_off = domcr_flags & XEN_DOMCTL_CDF_oos_off;
+d->arch.paging.shadow.oos_off = d->createflags & XEN_DOMCTL_CDF_oos_off;
 #endif
 d->arch.paging.shadow.pagetable_dying_op = 0;
 
diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h
index 6c7c4f5aa6..b5292696d2 100644
--- a/xen/include/asm-x86/hvm/domain.h
+++ b/xen/include/asm-x86/hvm/domain.h
@@ -156,7 +156,6 @@ struct hvm_domain {
 
 struct viridian_domain *viridian;
 
-bool_t hap_enabled;
 bool_t mem_sharing_enabled;
 bool_t qemu_mapcache_invalidate;
 bool_t is_s3_suspended;
@@ -195,11 +194,9 @@ struct hvm_domain {
 };
 };
 
-#ifdef CONFIG_HVM
-#define hap_enabled(d)  (is_hvm_domain(d) && (d)->arch.hvm.hap_enabled)
-#else
-#define hap_enabled(d)  ({(void)(d); false;})
-#endif
+#define hap_enabled(d) \
+(hvm_hap_supported() && is_hvm_domain(d) && \
+ evaluate_nospec(d->createflags & XEN_DOMCTL_CDF_hap))
 
 #endif /* __ASM_X86_HVM_DOMAIN_H__ */
 
diff --git a/xen/include/asm-x86/paging.h b/xen/include/asm-x86/paging.h
index cf57ca708d..ab7887f23c 100644
--- a/xen/include/asm-x86/paging.h
+++ b/xen/include/asm-x86/paging.h
@@ -207,7 +207,7 @@ void paging_vcpu_init(struct vcpu *v);
 
 /* Set up the paging-assistance-specific parts of a domain struct at
  * start of day.  Called for every domain from arch_domain_create() */
-int paging_domain_init(struct domain *d, unsigned int domcr_flags);
+int paging_domain_init(struct domain *d);
 
 /* Handler for paging-control ops: operations from user-space to enable
  * and disable ephemeral shadow modes (test mode and log-dirty mode) and
diff --git a/xen/include/asm-x86/shadow.h b/xen/include/asm-x86/shadow.h
index f29f0f652b..8ebb89c027 100644
--- a/xen/include/asm-x86/shadow.h
+++ b/xen/include/asm-x86/shadow.h
@@ -49,7 +49,7 @@
 
 /* Set up the shadow-specific parts of a domain struct at start of day.
  * Called from paging_domain_init(). */
-int shadow_domain_init(struct domain *d, unsigned int domcr_flags);
+int shadow_domain_init(struct domain *d);
 
 /* Setup the shadow-sp

[Xen-devel] [PATCH 0/6] stash domain create flags and then use them

[Paul Durrant]

This series starts with a patch to stash the domain create flags in struct
domain then then follows up with various clean-up patches that this
enables.

Paul Durrant (6):
  domain: stash xen_domctl_createdomain flags in struct domain
  domain: remove 'guest_type' field (and enum guest_type)
  x86/hvm/domain: remove the 'hap_enabled' flag
  x86/domain: remove the 'oos_off' flag
  domain: remove the 'is_xenstore' flag
  x86/domain: remove the 's3_integrity' flag

 xen/arch/x86/domain.c|  9 +
 xen/arch/x86/mm/paging.c |  4 ++--
 xen/arch/x86/mm/shadow/common.c  |  5 ++---
 xen/arch/x86/tboot.c |  2 +-
 xen/common/domain.c  | 11 +++
 xen/common/domctl.c  | 12 ++--
 xen/common/kernel.c  |  9 ++---
 xen/include/asm-x86/domain.h |  3 ---
 xen/include/asm-x86/hvm/domain.h |  9 +++--
 xen/include/asm-x86/paging.h |  2 +-
 xen/include/asm-x86/shadow.h |  2 +-
 xen/include/xen/sched.h  | 22 ++
 xen/include/xsm/dummy.h  |  2 +-
 13 files changed, 29 insertions(+), 63 deletions(-)
---
Cc: Andrew Cooper 
Cc: Daniel De Graaf 
Cc: Gang Wei 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Julien Grall 
Cc: Konrad Rzeszutek Wilk 
Cc: "Roger Pau Monné" 
Cc: Shane Wang 
Cc: Stefano Stabellini 
Cc: Tim Deegan 
Cc: Wei Liu 
-- 
2.20.1.2.gb21ebb671


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

Re: [Xen-devel] [PATCH v3 24/35] OvmfPkg/XenPlatformPei: Rework memory detection

[Anthony PERARD]

On Tue, Jul 23, 2019 at 11:42:07AM +0200, Roger Pau Monné wrote:
> On Mon, Jul 22, 2019 at 03:53:19PM +0100, Anthony PERARD wrote:
> > On Mon, Jul 15, 2019 at 04:15:21PM +0200, Roger Pau Monné wrote:
> > > On Thu, Jul 04, 2019 at 03:42:22PM +0100, Anthony PERARD wrote:
> > > You could maybe initialize this as a global to avoid having to issue
> > > a hypercall each time you need to get something from the memory map.
> > 
> > That function does that, it only make the hypercall once. (The hypercall
> > can only be made once anyway, the second time Xen doesn't return the
> > map.)
> 
> Why? I'm looking at the implementation in Xen of XENMEM_memory_map and
> I'm not sure I see how/why the hypercall can only be made once. AFAICT
> you should be able to call XENMEM_memory_map multiple times without
> issues, or else it's a bug somewhere.

:-(, I probably made a mistake when testing that. I tried again and
calling the hypercall serveral time gave the same result. Sorry for the
noise.

> > > > +}
> > > >  
> > > >  UINT32
> > > >  GetSystemMemorySizeBelow4gb (
> > > > @@ -105,6 +146,19 @@ GetSystemMemorySizeBelow4gb (
> > > >UINT8 Cmos0x34;
> > > >UINT8 Cmos0x35;
> > > >  
> > > > +  //
> > > > +  // In PVH case, there is no CMOS, we have to calculate the memory 
> > > > size
> > > > +  // from parsing the E820
> > > > +  //
> > > > +  if (XenPvhDetected ()) {
> > > 
> > > IIRC on HVM you can also get the memory map from the hypercall, in
> > > which case you could use the same code path for both HVM and PVH.
> > 
> > I think that wouldn't work because in my experiment, the hypercall would
> > only return the map the first time (at least on PVH). hvmloader already
> > make the hypercall so OVMF can't.
> 
> OK, I'm not sure the reason for this, as I said above I think this is
> a bug somewhere. You should be able to call XENMEM_memory_map multiple
> times.
> 
> > On the other hand, XenGetE820Map() return an E820 map, it doesn't matter
> > if it's the one passed by hvmloader, or the one we've got directly from
> > Xen. So I guess we could ignore what hvmloader have written in the CMOS
> 
> Hm, I'm not sure hvmloader uploads a new memory map to Xen (using
> XENMEM_set_memory_map) if it does any modifications to it. It should
> certainly do it, so that the guest OS gets the same memory map from
> the hypercall or from the firmware.

hvmloader doesn't call XENMEM_set_memory_map (I don't find that string
in the source code), also, I've tested again calling the get memory_map
hypercall in HVM guests and the e820 from hvmloader is different from
the one from the hypercall:

from hvmloader:
Type Mem  -  -> 000A
Type Res  - 000F -> 0010
Type Mem  - 0010 -> 3F6B3000
Type Res  - FC00 -> 1
from Xen:
Type Mem  - 0010 -> 3F80

> > > > +switch (Entry->Type) {
> > > > +case EfiAcpiAddressRangeMemory:
> > > > +  AddMemoryRangeHob (Base, End);
> > > > +  break;
> > > > +case EfiAcpiAddressRangeACPI:
> > > > +  //
> > > > +  // Ignore, OVMF should read the ACPI tables and provide them to 
> > > > linux
> > > > +  // from a different location.
> > > 
> > > Will OVMF also parse dynamic tables to check for references there?
> > 
> > I haven't looked at what OVMF does with the ACPI tables, but Linux seems
> > fine. I've compared the boot output of linux running as PVH vs booted
> > via OVMF. Beside the location of the table been different, the number of
> > table where the same, I don't remember other difference.
> 
> OK, what I find weird is that you seem to discard quite a lot of stuff
> from the original memory map, and then reconstruct it afterwards I
> assume?
> 
> It would seem safer to not discard regions from the memory map
> provided to OVMF, and instead just build on top of it. I would expect

OK, I'll add back the EfiAcpiAddressRangeACPI into the reserved regions.

> for example that OVMF will use some of the RAM regions on the memory
> map, and it should likely turn those areas from RAM into reserved
> regions.
> 
> > > > +  // error message: CpuDxe: IntersectMemoryDescriptor:
> > > > +  //desc [FC00, 1) type 1 cap 87026001
> > > > +  //conflicts with aperture [FEE0, FEE01000) cap 1
> > > >//
> > > > -  if (Entry->Type != EfiAcpiAddressRangeMemory) {
> > > > -continue;
> > > > +  if (!XenHvmloaderDetected ()) {
> > > > +AddReservedMemoryBaseSizeHob (Base, End - Base, FALSE);
> > > 
> > > This special casing for PVH looks weird, ideally we would like to use
> > > the same code path, or else it should be explicitly mentioned why PVH
> > > has diverging behaviour.
> > 
> > I think hvmloader is the issue rather than PVH. Here is part of the
> > "memory map" as found in hvmloader/config.h:
> > 
> >   /* Special BIOS mappings, etc. are allocated from here upwards... */
> >   #define RESERVED_MEMBASE  0xFC00
> >   /* NB. ACPI_INFO_P

Re: [Xen-devel] [PATCH 1/2] x86/iommu: avoid mapping the interrupt address range for hwdom

[Jan Beulich]

On 23.07.2019 17:48, Roger Pau Monne wrote:
> Current code only prevent mapping the lapic page into the guest
> physical memory map. Expand the range to be 0xFEEx_ as described
> in the Intel VTd specification section 3.13 "Handling Requests to
> Interrupt Address Range".

Right, and it being device side accesses that are of interest here,
the LAPIC address range (visible to the CPU only) shouldn't even
matter. Hence after some back and forth with myself I agree that
you remove the entire comment there.

> AMD also lists this address range in the AMD SR5690 Databook, section
> 2.4.4 "MSI Interrupt Handling and MSI to HT Interrupt Conversion".

Which raises the question about yet another patch to also exclude
the HT range.

> Requested-by: Andrew Cooper 
> Signed-off-by: Roger Pau Monné 

Reviewed-by: Jan Beulich 

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

Re: [Xen-devel] [PATCH 2/2] x86/iommu: avoid mapping the APIC configuration space for hwdom

[Jan Beulich]

On 23.07.2019 17:48, Roger Pau Monne wrote:
> Current code only prevents mapping the io-apic page into the guest
> physical memory map. Expand the range to be 0xFECx_ as described
> in the Intel 3 Series Chipset Datasheet section 3.3.1 "APIC
> Configuration Space (FEC0_h–FECF_h)".
> 
> AMD also lists this address range in the AMD SR5690 Databook, section
> 2.4.2 "Non-SB IOAPIC Support".

But that's chipset specific. I don't think we can blindly assume
this range. Just in case one small remark on the change itself as
well:

> @@ -229,10 +229,9 @@ static bool __hwdom_init hwdom_iommu_map(const struct 
> domain *d,
>   /* Check that it doesn't overlap with the Interrupt Address Range. */
>   if ( pfn >= 0xfee00 && pfn <= 0xfeeff )
>   return false;
> -/* ... or the IO-APIC */
> -for ( i = 0; has_vioapic(d) && i < d->arch.hvm.nr_vioapics; i++ )
> -if ( pfn == PFN_DOWN(domain_vioapic(d, i)->base_address) )
> -return false;
> +/* ... or the APIC Configuration Space. */
> +if ( pfn >= 0xfec00 && pfn <= 0xfecff )
> +return false;

Despite the chipset documentation calling it just APIC, in our
code I think it would be better if a connection to IO-APIC was
made, to avoid ambiguity.

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

Re: [Xen-devel] [PATCH 2/2] x86/iommu: avoid mapping the APIC configuration space for hwdom

[Andrew Cooper]

On 23/07/2019 17:09, Jan Beulich wrote:
> On 23.07.2019 17:48, Roger Pau Monne wrote:
>> Current code only prevents mapping the io-apic page into the guest
>> physical memory map. Expand the range to be 0xFECx_ as described
>> in the Intel 3 Series Chipset Datasheet section 3.3.1 "APIC
>> Configuration Space (FEC0_h–FECF_h)".
>>
>> AMD also lists this address range in the AMD SR5690 Databook, section
>> 2.4.2 "Non-SB IOAPIC Support".
> But that's chipset specific. I don't think we can blindly assume
> this range.

The IO-APIC has always lived in that region since its introduction, and
the location isn't even configurable on newer chipsets (If I've read the
SAD routing rules in Skylake correctly.  All that can be configured is
multiple IO-APICs being mapped adjacent to each other.)

While this isn't the inbound MSI range (and definitely fixed in the
architecture), it isn't plausibly going to change.

~Andrew

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

Re: [Xen-devel] [PATCH] vunmap: let vunmap align virtual address by itself

[Julien Grall]

Hi Jan,

On 23/07/2019 10:12, Jan Beulich wrote:

On 23.07.2019 10:48, Andrii Anisov wrote:

Julien, Jan, Andrew,

The problem addressed by [1] causes random ARM64 boot fails dependent on 
hypervisor code changes.
Yet more generic solution was requested by Andrew and supported by Julien [2].

How to proceed with this particular patch?
As I understand, Jan doubts we should move page alignment to vunmap(), while 
Julien and Andrew wanted the commit message clarification.
Can we have an agreement on approach here?

[1] https://lists.xenproject.org/archives/html/xen-devel/2019-07/msg01167.html
[2] https://lists.xenproject.org/archives/html/xen-devel/2019-07/msg01129.html


First of all, let me quote Linux'es code:

static void __vunmap(const void *addr, int deallocate_pages)
{
struct vm_struct *area;

if (!addr)
return;

if (WARN(!PAGE_ALIGNED(addr), "Trying to vfree() bad address (%p)\n",
addr))
return;

As long as we aim to have a reasonable level of compatibility of
similar interfaces, we should not go the suggested route.


Well, it is more likely to have Linux code moving to Xen compare to the 
opposite. So the change suggested is still compatible as we don't restrict 
anything but just open more flexibility.




Beyond that I continue to be of the opinion that it should be
all-or-nothing: Any pointer pointing anywhere at or inside the
region should be accepted, or just the one pointing precisely at
the start.
That's fine, but we can still achieve this step by step... Handling unaligned 
address is quite easy.


Cheers,

--
Julien Grall

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

[Xen-devel] [xen-unstable test] 139259: tolerable FAIL - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 139239
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 139239
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 139239
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 139239
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 139239
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 139239
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 139239
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 139239
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 139239
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim12 guest-start  fail   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-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail 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-xl-thunderx 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  14 saverestore-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-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  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-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-credit1  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 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-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-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-amd64-xl-qemuu-win10-i386 10 windows-installfail 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-i386-xl-qemut-win10-i386 10 windows-install fail never pass

version targeted for testing:
 xen  f7f7cd5c585ab2d6f4bbc17b8fbec52dde1a5715
baseline version:
 xen  66d11b9c1281ac133b92b3e6dd5b5e1c2abea7bf

Last test of basis   139239  2019-07-21 18:39:50 Z1 days
Testing same since   139259  2019-07-22 16:03:28 Z1 days1 attempts


People who touch

Re: [Xen-devel] [BUG] After upgrade to Xen 4.12.0 iommu=no-igfx

[Roman Shaposhnik]

Hi Roger!

I applied your patch, removed no-igfx and I still see the original
problem. Please let me know what other logs/debugs would you need at
this point.

Btw, just to make it clear what patch got applied I'm attaching it to
this email.

Oh, and it seems that that https://downloads.xenproject.org/ SSL
certificate expired yesterday -- perhaps somebody can take a look at
that.

Thanks,
Roman.

On Mon, Jul 22, 2019 at 4:47 PM Andrew Cooper  wrote:
>
> On 23/07/2019 00:36, Roman Shaposhnik wrote:
> > Hi Everyone!
> >
> > Thanks a million for an extremely quick turnaround. I am in my lab
> > again next to the box in question.
> >
> > Should I go ahead and test the latest patch or wait for the official
> > one to be submitted?
> >
> > Thanks,
> > Roman.
>
> Use this patch to test with.  Roger forgot to CC you on the official
> one, but the code changes are identical.
>
> ~Andrew


01-iommu-mappings.patch
Description: Binary data
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [BUG] After upgrade to Xen 4.12.0 iommu=no-igfx

[Andrew Cooper]

On 23/07/2019 18:32, Roman Shaposhnik wrote:
> Hi Roger!
>
> I applied your patch, removed no-igfx and I still see the original
> problem. Please let me know what other logs/debugs would you need at
> this point.

Please can you collect a full boot log with iommu=debug

~Andrew

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

Re: [Xen-devel] [BUG] After upgrade to Xen 4.12.0 iommu=no-igfx

[Andrew Cooper]

On 23/07/2019 18:48, Roman Shaposhnik wrote:
> On Tue, Jul 23, 2019 at 10:35 AM Andrew Cooper
>  wrote:
>> On 23/07/2019 18:32, Roman Shaposhnik wrote:
>>> Hi Roger!
>>>
>>> I applied your patch, removed no-igfx and I still see the original
>>> problem. Please let me know what other logs/debugs would you need at
>>> this point.
>> Please can you collect a full boot log with iommu=debug
> How long of an output should I expect when iommu=debug is enabled?
> I've just enabled it and I'm looking at what appears to be an endless
> scroll of debug info.
>
> This is all I see for the good 5 minutes at this point. Culminating with:
> (XEN)   (XEN) APIC error on CPU0: 40(00)
>
> and a failure to boot.
>
> Note that this is still without no-igfx
>
> I'm attaching the tail end of this log.

Sadly, what is useful is the head of the log, before it starts
complaining loudly about every DMA fault.

~Andrew

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

Re: [Xen-devel] Xen Hypervisor porting on Raspberry Pi 3B+/4

[Roman Shaposhnik]

It would be great to have Xen running on RPi, but I have to wonder: is
it now possible to workaround RPi limitations of how GPU boots?
   https://www.raspberrypi.org/forums/viewtopic.php?t=187086#p1206487

I thought that this is completely locked, proprietary bcm2837 code
that Xen can't do much of anything about.

Thanks,
Roman.


On Tue, Jul 23, 2019 at 1:55 AM Sushant Bhangale
 wrote:
>
> Hi,
>
>
>
> Awaited for your input.
>
>
>
> Regards,
>
> Sushant
>
>
>
> From: Sushant Bhangale
> Sent: Friday, July 12, 2019 7:52 PM
> To: xen-devel@lists.xenproject.org
> Cc: Pranav Paralikar ; Nikhil Wadke 
> ; sstabell...@kernel.org; julien.gr...@arm.com; 
> lars.ku...@citrix.com
> Subject: Xen Hypervisor porting on Raspberry Pi 3B+/4
>
>
>
> Hi,
>
>
>
> We are planning to port Xen Hypervisor on Raspberry PI 3B+(A53 Processor) OR 
> Raspberry PI 4 (A72 Processor). For this purpose we find out the below tools,
>
>
>
> Tools
>
> Versions
>
> Path
>
> Cross Compiler
>
> gcc-linaro-arm-none-eabi-4.9-2014.09_linux
>
> .
>
> gcc-linaro-7.2.1-2017.11-x86_64_aarch64-linux-gnu
>
> .
>
> arm-linux-gnueabi-gcc
>
> .
>
> Bootloader
>
> U-Boot
>
> https://github.com/u-boot/u-boot
>
> Linux Kernel
>
> v3.18-rc15
>
> https://wiki.xenproject.org/wiki/Xen_ARM_with_Virtualization_Extensions
>
> linux-4.2.tar.xz
>
> https://www.kernel.org/pub/linux/kernel/v4.x/linux-4.2.tar.xz
>
> Root Filesystem
>
> BusyBox-1.30.1 or Older
>
> https://busybox.net/
>
> Xen Hypervisor Source code
>
> Xen Hypervisor (v4.12 Stable version)
>
> https://github.com/bkrepo/xen.git
>
> Processor 1
>
> Broadcom BCM5871X (A53)
>
> Raspberry Pi 3B / 3B+
>
>  Processor 2
>
> Broadcom BCM2711 (A72)
>
> Raspberry Pi 4
>
>
>
> Request you to let us know whether above selected tools in the table is OK or 
> NOT.
>
>
>
> Also, if any document related to porting of Xen hypervisor on Raspberry PI 
> 3B+(A53 Processor) OR Raspberry PI 4 (A72 Processor) is available with you, 
> please share the same.
>
>
>
> Looking forward for your response.
>
>
>
> Regards,
>
> Sushant Bhangale
>
> Senior Engineer - EDP
>
> L&T TECHNOLOGY SERVICES LIMITED
>
> 8th Floor Building No,1,
>
> Thane Belapur Road, Mindspace
>
> Airoli, Navi Mumbai :India 400708
> 
>
>
>
> Tel: +91 22  6105 8289 | Mobile: +91 76 2033 0707
>
> www.Ltts.com
>
>
>
> L&T Technology Services Ltd
>
> www.LTTS.com
>
> This Email may contain confidential or privileged information for the 
> intended recipient (s). If you are not the intended recipient, please do not 
> use or disseminate the information, notify the sender and delete it from your 
> system.
>
> --
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to xenprojecthelp+unsubscr...@linuxfoundation.org.
>
> L&T Technology Services Ltd
>
> www.LTTS.com
>
> This Email may contain confidential or privileged information for the 
> intended recipient (s). If you are not the intended recipient, please do not 
> use or disseminate the information, notify the sender and delete it from your 
> system.
>
> ___
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel

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

Re: [Xen-devel] [BUG] After upgrade to Xen 4.12.0 iommu=no-igfx

[Roman Shaposhnik]

On Tue, Jul 23, 2019 at 10:50 AM Andrew Cooper
 wrote:
>
> On 23/07/2019 18:48, Roman Shaposhnik wrote:
> > On Tue, Jul 23, 2019 at 10:35 AM Andrew Cooper
> >  wrote:
> >> On 23/07/2019 18:32, Roman Shaposhnik wrote:
> >>> Hi Roger!
> >>>
> >>> I applied your patch, removed no-igfx and I still see the original
> >>> problem. Please let me know what other logs/debugs would you need at
> >>> this point.
> >> Please can you collect a full boot log with iommu=debug
> > How long of an output should I expect when iommu=debug is enabled?
> > I've just enabled it and I'm looking at what appears to be an endless
> > scroll of debug info.
> >
> > This is all I see for the good 5 minutes at this point. Culminating with:
> > (XEN)   (XEN) APIC error on CPU0: 40(00)
> >
> > and a failure to boot.
> >
> > Note that this is still without no-igfx
> >
> > I'm attaching the tail end of this log.
>
> Sadly, what is useful is the head of the log, before it starts
> complaining loudly about every DMA fault.

No worries. Take a look at the head of the log attached.

Btw, I'm kind of curious why iommu=debug would actually make it crash

Thanks,
Roman.
0x:0x00:0x02.0x0: ROM: 0x1 bytes at 0x8968d018
0x:0x02:0x00.0x0: ROM: 0x1 bytes at 0x89640018
0x:0x00:0x1f.0x6: ROM: 0x10c00 bytes at 0x8962f018
 Xen 4.12.0
(XEN) Xen version 4.12.0 (@) (gcc (Alpine 6.4.0) 6.4.0) debug=n  Tue Jul 23 
17:15:48 UTC 2019
(XEN) Latest ChangeSet:
(XEN) Bootloader: GRUB 2.03
(XEN) Command line: com1=115200,8n1 console=com1,vga iommu=debug 
dom0_mem=1024M,max:1024M dom0_max_vcpus=1 dom0_vcpus_pin smt=false
(XEN) Xen image load base address: 0x8800
(XEN) Video information:
(XEN)  VGA is graphics mode 1680x1050, 32 bpp
(XEN) Disc information:
(XEN)  Found 0 MBR signatures
(XEN)  Found 1 EDD information structures
(XEN) EFI RAM map:
(XEN)   - 00058000 (usable)
(XEN)  00058000 - 00059000 (reserved)
(XEN)  00059000 - 0009f000 (usable)
(XEN)  0009f000 - 000a (reserved)
(XEN)  0010 - 8648a000 (usable)
(XEN)  8648a000 - 8648b000 (ACPI NVS)
(XEN)  8648b000 - 864b5000 (reserved)
(XEN)  864b5000 - 8c224000 (usable)
(XEN)  8c224000 - 8c528000 (reserved)
(XEN)  8c528000 - 8c736000 (usable)
(XEN)  8c736000 - 8cea7000 (ACPI NVS)
(XEN)  8cea7000 - 8d2ff000 (reserved)
(XEN)  8d2ff000 - 8d30 (usable)
(XEN)  8d30 - 8d40 (reserved)
(XEN)  e000 - f000 (reserved)
(XEN)  fe00 - fe011000 (reserved)
(XEN)  fec0 - fec01000 (reserved)
(XEN)  fee0 - fee01000 (reserved)
(XEN)  ff00 - 0001 (reserved)
(XEN)  0001 - 00016e00 (usable)
(XEN) ACPI: RSDP 8CE49000, 0024 (r2 ALASKA)
(XEN) ACPI: XSDT 8CE490A8, 00CC (r1 ALASKA   A M I   1072009 AMI 10013)
(XEN) ACPI: FACP 8CE6C370, 010C (r5 ALASKA   A M I   1072009 AMI 10013)
(XEN) ACPI: DSDT 8CE49208, 23167 (r2 ALASKA   A M I   1072009 INTL 20120913)
(XEN) ACPI: FACS 8CE8EF80, 0040
(XEN) ACPI: APIC 8CE6C480, 0084 (r3 ALASKA   A M I   1072009 AMI 10013)
(XEN) ACPI: FPDT 8CE6C508, 0044 (r1 ALASKA   A M I   1072009 AMI 10013)
(XEN) ACPI: FIDT 8CE6C550, 009C (r1 ALASKA   A M I   1072009 AMI 10013)
(XEN) ACPI: MCFG 8CE6C5F0, 003C (r1 ALASKA   A M I   1072009 MSFT   97)
(XEN) ACPI: HPET 8CE6C630, 0038 (r1 ALASKA   A M I   1072009 AMI.5000B)
(XEN) ACPI: LPIT 8CE6C668, 0094 (r1 INTEL   SKL-ULT0 MSFT   5F)
(XEN) ACPI: SSDT 8CE6C700, 0248 (r2 INTEL  sensrhub0 INTL 20120913)
(XEN) ACPI: SSDT 8CE6C948, 2BAE (r2 INTEL  PtidDevc 1000 INTL 20120913)
(XEN) ACPI: SSDT 8CE6F4F8, 0BE3 (r2 INTEL  Ther_Rvp 1000 INTL 20120913)
(XEN) ACPI: SSDT 8CE700E0, 04A3 (r2 INTEL zpodd 1000 INTL 20120913)
(XEN) ACPI: DBGP 8CE70588, 0034 (r1 INTEL  0 MSFT   5F)
(XEN) ACPI: DBG2 8CE705C0, 0054 (r0 INTEL  0 MSFT   5F)
(XEN) ACPI: SSDT 8CE70618, 06E9 (r2  INTEL xh_rvp070 INTL 20120913)
(XEN) ACPI: SSDT 8CE70D08, 547E (r2 SaSsdt  SaSsdt  3000 INTL 20120913)
(XEN) ACPI: UEFI 8CE76188, 0042 (r10 0)
(XEN) ACPI: SSDT 8CE761D0, 0E73 (r2 CpuRef  CpuSsdt 3000 INTL 20120913)
(XEN) ACPI: BGRT 8CE77048, 0038 (r1 ALASKA   A M I   1072009 AMI 10013)
(XEN) ACPI: DMAR 8CE77080, 00A8 (r1 INTEL  SKL 1 INTL1)
(XEN) ACPI: TPM2 8CE77128, 0034 (r3Tpm2Tabl1 AMI 0)
(XEN) ACPI: ASF! 8CE77160, 00A5 (r32 INTEL   HCG1 TFSMF4240)
(XEN) System RAM: 4003MB (4099736kB)
(XEN) Domain heap initialised
(XEN) ACPI: 32/64X FACS address mismatch in FADT - 8ce8ef80/, 
using 32
(XEN) IOAPIC[0]: apic_id 2, version 32, address 0xfec0, GSI 0-119
(XEN) Enabling APIC mode:  Flat.  Using 1 I/O 

Re: [Xen-devel] [BUG] After upgrade to Xen 4.12.0 iommu=no-igfx

[Andrew Cooper]

On 23/07/2019 18:58, Roman Shaposhnik wrote:
> On Tue, Jul 23, 2019 at 10:50 AM Andrew Cooper
>  wrote:
>> On 23/07/2019 18:48, Roman Shaposhnik wrote:
>>> On Tue, Jul 23, 2019 at 10:35 AM Andrew Cooper
>>>  wrote:
 On 23/07/2019 18:32, Roman Shaposhnik wrote:
> Hi Roger!
>
> I applied your patch, removed no-igfx and I still see the original
> problem. Please let me know what other logs/debugs would you need at
> this point.
 Please can you collect a full boot log with iommu=debug
>>> How long of an output should I expect when iommu=debug is enabled?
>>> I've just enabled it and I'm looking at what appears to be an endless
>>> scroll of debug info.
>>>
>>> This is all I see for the good 5 minutes at this point. Culminating with:
>>> (XEN)   (XEN) APIC error on CPU0: 40(00)
>>>
>>> and a failure to boot.
>>>
>>> Note that this is still without no-igfx
>>>
>>> I'm attaching the tail end of this log.
>> Sadly, what is useful is the head of the log, before it starts
>> complaining loudly about every DMA fault.
> No worries. Take a look at the head of the log attached.
>
> Btw, I'm kind of curious why iommu=debug would actually make it crash

The system is rather sickly, and is debugging at a rate slower than
incoming faults, which is going to starve whichever CPU is taking the
IOMMU interrupt.

I wouldn't worry about the APIC error now.

Curiously, there is one single intremap error on boot, which is likely
unrelated.

(XEN) [VT-D]INTR-REMAP: Request device [:f0:1f.0] fault index 0, iommu reg 
= 82c0008e

(XEN) [VT-D]INTR-REMAP: reason 22 - Present field in the IRTE entry is clear


This will be irq0 from the IO-APIC.

Can you try booting following the guidance from

(XEN) [VT-D]found ACPI_DMAR_RMRR:

(XEN) [VT-D]  RMRR address range 8d80..8fff not in reserved memory; 
need "iommu_inclusive_mapping=1"?

(XEN) [VT-D] endpoint: :00:02.0


which I noted on my first reply?  Given that Rogers patch didn't help,
something else is going on.

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

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

[osstest service owner]

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

Failures :-/ but no regressions.

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

version targeted for testing:
 xen  52fc4aaf1613e49d018bf3c5b1899b131ee2f417
baseline version:
 xen  0d10a77cc98dda1b8f9a96954149a872f46048c9

Last test of basis   139282  2019-07-23 13:00:45 Z0 days
Testing same since   139290  2019-07-23 16:00:40 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Brian Woods 
  Jan Beulich 
  Roger Pau Monné 

jobs:
 build-arm64-xsm  pass
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-arm64-arm64-xl-xsm  pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-amd64-libvirt pass



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

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

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

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


Pushing revision :

To xenbits.xen.org:/home/xen/git/xen.git
   0d10a77cc9..52fc4aaf16  52fc4aaf1613e49d018bf3c5b1899b131ee2f417 -> smoke

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

[Xen-devel] [PATCH v2 2/2] xen: merge temporary vcpu pinning scenarios

[Juergen Gross]

Today there are two scenarios which are pinning vcpus temporarily to
a single physical cpu:

- wait_event() handling
- vcpu_pin_override() handling

Each of those cases are handled independently today using their own
temporary cpumask to save the old affinity settings.

The two cases can be combined as the first case will only pin a vcpu to
the physical cpu it is already running on, while vcpu_pin_override() is
allowed to fail.

So merge the two temporary pinning scenarios by only using one cpumask
and a per-vcpu bitmask for specifying which of the scenarios is
currently active (they are allowed to nest).

Note that we don't need to call domain_update_node_affinity() as we
are only pinning for a brief period of time.

Signed-off-by: Juergen Gross 
---
V2:
- removed the NMI/MCE case
- rename vcpu_set_tmp_affinity() (Jan Beulich)
- remove vcpu_pin_override() wrapper (Andrew Cooper)
- current -> curr (Jan Beulich, Andrew Cooper)
- make cpu parameter unsigned int (Jan Beulich)
- add comment (Dario Faggioli)
---
 xen/common/domain.c |  1 +
 xen/common/domctl.c |  2 +-
 xen/common/schedule.c   | 46 --
 xen/common/wait.c   | 30 ++
 xen/include/xen/sched.h |  8 +---
 5 files changed, 49 insertions(+), 38 deletions(-)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index bc56a51815..e8e850796e 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -1267,6 +1267,7 @@ int vcpu_reset(struct vcpu *v)
 v->async_exception_mask = 0;
 memset(v->async_exception_state, 0, sizeof(v->async_exception_state));
 #endif
+v->affinity_broken = 0;
 clear_bit(_VPF_blocked, &v->pause_flags);
 clear_bit(_VPF_in_reset, &v->pause_flags);
 
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 72a44953d0..fa260ce5fb 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -654,7 +654,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
u_domctl)
 
 /* Undo a stuck SCHED_pin_override? */
 if ( vcpuaff->flags & XEN_VCPUAFFINITY_FORCE )
-vcpu_pin_override(v, -1);
+vcpu_temporary_affinity(v, NR_CPUS, VCPU_AFFINITY_OVERRIDE);
 
 ret = 0;
 
diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index 349f9624f5..508176a142 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -1106,43 +1106,59 @@ void watchdog_domain_destroy(struct domain *d)
 kill_timer(&d->watchdog_timer[i]);
 }
 
-int vcpu_pin_override(struct vcpu *v, int cpu)
+/*
+ * Pin a vcpu temporarily to a specific CPU (or restore old pinning state if
+ * cpu is NR_CPUS).
+ * Temporary pinning can be done due to two reasons, which may be nested:
+ * - VCPU_AFFINITY_OVERRIDE (requested by guest): is allowed to fail in case
+ *   of a conflict (e.g. in case cpupool doesn't include requested CPU, or
+ *   another conflicting temporary pinning is already in effect.
+ * - VCPU_AFFINITY_WAIT (called by wait_event(): only used to pin vcpu to the
+ *   CPU it is just running on. Can't fail if used properly.
+ */
+int vcpu_temporary_affinity(struct vcpu *v, unsigned int cpu, uint8_t reason)
 {
 spinlock_t *lock;
 int ret = -EINVAL;
+bool migrate;
 
 lock = vcpu_schedule_lock_irq(v);
 
-if ( cpu < 0 )
+if ( cpu == NR_CPUS )
 {
-if ( v->affinity_broken )
+if ( v->affinity_broken & reason )
 {
-sched_set_affinity(v, v->cpu_hard_affinity_saved, NULL);
-v->affinity_broken = 0;
 ret = 0;
+v->affinity_broken &= ~reason;
 }
+if ( !ret && !v->affinity_broken )
+sched_set_affinity(v, v->cpu_hard_affinity_saved, NULL);
 }
 else if ( cpu < nr_cpu_ids )
 {
-if ( v->affinity_broken )
+if ( (v->affinity_broken & reason) ||
+ (v->affinity_broken && v->processor != cpu) )
 ret = -EBUSY;
 else if ( cpumask_test_cpu(cpu, VCPU2ONLINE(v)) )
 {
-cpumask_copy(v->cpu_hard_affinity_saved, v->cpu_hard_affinity);
-v->affinity_broken = 1;
-sched_set_affinity(v, cpumask_of(cpu), NULL);
+if ( !v->affinity_broken )
+{
+cpumask_copy(v->cpu_hard_affinity_saved, v->cpu_hard_affinity);
+sched_set_affinity(v, cpumask_of(cpu), NULL);
+}
+v->affinity_broken |= reason;
 ret = 0;
 }
 }
 
-if ( ret == 0 )
+migrate = !ret && !cpumask_test_cpu(v->processor, v->cpu_hard_affinity);
+if ( migrate )
 vcpu_migrate_start(v);
 
 vcpu_schedule_unlock_irq(lock, v);
 
-domain_update_node_affinity(v->domain);
-
-vcpu_migrate_finish(v);
+if ( migrate )
+vcpu_migrate_finish(v);
 
 return ret;
 }
@@ -1258,6 +1274,7 @@ ret_t do_sched_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) 
arg)
 case SCHEDOP_pin_override:
 {
 struct sched_pin_over

[Xen-devel] [PATCH v2 0/2] xen: enhance temporary vcpu pinning

[Juergen Gross]

While trying to handle temporary vcpu pinnings in a sane way in my
core scheduling series I found a nice way to simplify the temporary
pinning cases.

I'm sending the two patches independently from my core scheduling
series as they should be considered even without core scheduling.

Changes in V2:
- original patch 1 dropped, as already applied
- new patch 1 removing dead coding and unneeded pinning
- addressed various comments in patch 2

Juergen Gross (2):
  xen/x86: cleanup unused NMI/MCE code
  xen: merge temporary vcpu pinning scenarios

 xen/arch/x86/pv/traps.c| 88 --
 xen/arch/x86/traps.c   | 10 +
 xen/common/domain.c|  4 +-
 xen/common/domctl.c|  2 +-
 xen/common/schedule.c  | 46 +++---
 xen/common/wait.c  | 30 +-
 xen/include/asm-x86/pv/traps.h |  8 ++--
 xen/include/asm-x86/softirq.h  |  2 +-
 xen/include/xen/sched.h| 10 ++---
 9 files changed, 72 insertions(+), 128 deletions(-)

-- 
2.16.4


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

[Xen-devel] [PATCH v2 1/2] xen/x86: cleanup unused NMI/MCE code

[Juergen Gross]

pv_raise_interrupt() is only called for NMIs these days, so the MCE
specific part can be removed. Rename pv_raise_interrupt() to
pv_raise_nmi() and NMI_MCE_SOFTIRQ to NMI_SOFTIRQ.

Additionally there is no need to pin the vcpu the NMI is delivered
to, that is a leftover of (already removed) MCE handling. So remove
the pinning, too.

Signed-off-by: Juergen Gross 
---
 xen/arch/x86/pv/traps.c| 88 --
 xen/arch/x86/traps.c   | 10 +
 xen/common/domain.c|  3 --
 xen/include/asm-x86/pv/traps.h |  8 ++--
 xen/include/asm-x86/softirq.h  |  2 +-
 xen/include/xen/sched.h|  2 -
 6 files changed, 23 insertions(+), 90 deletions(-)

diff --git a/xen/arch/x86/pv/traps.c b/xen/arch/x86/pv/traps.c
index 1740784ff2..9436c80047 100644
--- a/xen/arch/x86/pv/traps.c
+++ b/xen/arch/x86/pv/traps.c
@@ -136,47 +136,21 @@ bool set_guest_nmi_trapbounce(void)
 return !null_trap_bounce(curr, tb);
 }
 
-struct softirq_trap {
-struct domain *domain;   /* domain to inject trap */
-struct vcpu *vcpu;   /* vcpu to inject trap */
-unsigned int processor;  /* physical cpu to inject trap */
-};
+static DEFINE_PER_CPU(struct vcpu *, softirq_nmi_vcpu);
 
-static DEFINE_PER_CPU(struct softirq_trap, softirq_trap);
-
-static void nmi_mce_softirq(void)
+static void nmi_softirq(void)
 {
 unsigned int cpu = smp_processor_id();
-struct softirq_trap *st = &per_cpu(softirq_trap, cpu);
-
-BUG_ON(st->vcpu == NULL);
-
-/*
- * Set the tmp value unconditionally, so that the check in the iret
- * hypercall works.
- */
-cpumask_copy(st->vcpu->cpu_hard_affinity_tmp,
- st->vcpu->cpu_hard_affinity);
+struct vcpu **v_ptr = &per_cpu(softirq_nmi_vcpu, cpu);
 
-if ( (cpu != st->processor) ||
- (st->processor != st->vcpu->processor) )
-{
-
-/*
- * We are on a different physical cpu.  Make sure to wakeup the vcpu on
- * the specified processor.
- */
-vcpu_set_hard_affinity(st->vcpu, cpumask_of(st->processor));
-
-/* Affinity is restored in the iret hypercall. */
-}
+BUG_ON(*v_ptr == NULL);
 
 /*
- * Only used to defer wakeup of domain/vcpu to a safe (non-NMI/MCE)
+ * Only used to defer wakeup of domain/vcpu to a safe (non-NMI)
  * context.
  */
-vcpu_kick(st->vcpu);
-st->vcpu = NULL;
+vcpu_kick(*v_ptr);
+*v_ptr = NULL;
 }
 
 void __init pv_trap_init(void)
@@ -189,50 +163,22 @@ void __init pv_trap_init(void)
 _set_gate(idt_table + LEGACY_SYSCALL_VECTOR, SYS_DESC_trap_gate, 3,
   &int80_direct_trap);
 
-open_softirq(NMI_MCE_SOFTIRQ, nmi_mce_softirq);
+open_softirq(NMI_SOFTIRQ, nmi_softirq);
 }
 
-int pv_raise_interrupt(struct vcpu *v, uint8_t vector)
+int pv_raise_nmi(struct vcpu *v)
 {
-struct softirq_trap *st = &per_cpu(softirq_trap, smp_processor_id());
+struct vcpu **v_ptr = &per_cpu(softirq_nmi_vcpu, smp_processor_id());
 
-switch ( vector )
+if ( cmpxchgptr(v_ptr, NULL, v) )
+return -EBUSY;
+if ( !test_and_set_bool(v->nmi_pending) )
 {
-case TRAP_nmi:
-if ( cmpxchgptr(&st->vcpu, NULL, v) )
-return -EBUSY;
-if ( !test_and_set_bool(v->nmi_pending) )
-{
-st->domain = v->domain;
-st->processor = v->processor;
-
-/* Not safe to wake up a vcpu here */
-raise_softirq(NMI_MCE_SOFTIRQ);
-return 0;
-}
-st->vcpu = NULL;
-break;
-
-case TRAP_machine_check:
-if ( cmpxchgptr(&st->vcpu, NULL, v) )
-return -EBUSY;
-
-/*
- * We are called by the machine check (exception or polling) handlers
- * on the physical CPU that reported a machine check error.
- */
-if ( !test_and_set_bool(v->mce_pending) )
-{
-st->domain = v->domain;
-st->processor = v->processor;
-
-/* not safe to wake up a vcpu here */
-raise_softirq(NMI_MCE_SOFTIRQ);
-return 0;
-}
-st->vcpu = NULL;
-break;
+/* Not safe to wake up a vcpu here */
+raise_softirq(NMI_SOFTIRQ);
+return 0;
 }
+*v_ptr = NULL;
 
 /* Delivery failed */
 return -EIO;
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 25b4b47e5e..08d7edc568 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1600,14 +1600,6 @@ void async_exception_cleanup(struct vcpu *curr)
 if ( !curr->async_exception_mask )
 return;
 
-/* Restore affinity.  */
-if ( !cpumask_empty(curr->cpu_hard_affinity_tmp) &&
- !cpumask_equal(curr->cpu_hard_affinity_tmp, curr->cpu_hard_affinity) )
-{
-vcpu_set_hard_affinity(curr, curr->cpu_hard_affinity_tmp);
-cpumask_clear(curr->cpu_hard_affinity_tmp);
-}
-
 if ( !(curr->async_exception_mask & (curr->async_exception_mask - 1)) )
 tr

Re: [Xen-devel] [BUG] After upgrade to Xen 4.12.0 iommu=no-igfx

[Roman Shaposhnik]

On Tue, Jul 23, 2019 at 11:12 AM Andrew Cooper
 wrote:
>
> On 23/07/2019 18:58, Roman Shaposhnik wrote:
>
> On Tue, Jul 23, 2019 at 10:50 AM Andrew Cooper
>  wrote:
>
> On 23/07/2019 18:48, Roman Shaposhnik wrote:
>
> On Tue, Jul 23, 2019 at 10:35 AM Andrew Cooper
>  wrote:
>
> On 23/07/2019 18:32, Roman Shaposhnik wrote:
>
> Hi Roger!
>
> I applied your patch, removed no-igfx and I still see the original
> problem. Please let me know what other logs/debugs would you need at
> this point.
>
> Please can you collect a full boot log with iommu=debug
>
> How long of an output should I expect when iommu=debug is enabled?
> I've just enabled it and I'm looking at what appears to be an endless
> scroll of debug info.
>
> This is all I see for the good 5 minutes at this point. Culminating with:
> (XEN)   (XEN) APIC error on CPU0: 40(00)
>
> and a failure to boot.
>
> Note that this is still without no-igfx
>
> I'm attaching the tail end of this log.
>
> Sadly, what is useful is the head of the log, before it starts
> complaining loudly about every DMA fault.
>
> No worries. Take a look at the head of the log attached.
>
> Btw, I'm kind of curious why iommu=debug would actually make it crash
>
>
> The system is rather sickly, and is debugging at a rate slower than incoming 
> faults, which is going to starve whichever CPU is taking the IOMMU interrupt.
>
> I wouldn't worry about the APIC error now.

Got it. Makes sense.

> Curiously, there is one single intremap error on boot, which is likely 
> unrelated.
>
> (XEN) [VT-D]INTR-REMAP: Request device [:f0:1f.0] fault index 0, iommu 
> reg = 82c0008e
>
> (XEN) [VT-D]INTR-REMAP: reason 22 - Present field in the IRTE entry is clear
>
>
> This will be irq0 from the IO-APIC.
>
> Can you try booting following the guidance from
>
> (XEN) [VT-D]found ACPI_DMAR_RMRR:
>
> (XEN) [VT-D]  RMRR address range 8d80..8fff not in reserved memory; 
> need "iommu_inclusive_mapping=1"?
>
> (XEN) [VT-D] endpoint: :00:02.0
>
>
> which I noted on my first reply?  Given that Rogers patch didn't help, 
> something else is going on.

Sorry -- missed that option the first time around.

Interestingly enough, adding iommu_inclusive_mapping=1 AND iommu=debug
booted the system just fine.

There are no issues with screen.

I am attaching a full log.

Btw, my understanding is that this may point to a BIOS issue. Which
would be fair conclusion, but I've got to wonder why Xen 4.11 didn't
seem to be susceptible to this BIOS issue.

Thanks,
Roman.
0x:0x00:0x02.0x0: ROM: 0x1 bytes at 0x8968d018
0x:0x02:0x00.0x0: ROM: 0x1 bytes at 0x89640018
0x:0x00:0x1f.0x6: ROM: 0x10c00 bytes at 0x8962f018
 Xen 4.12.0
(XEN) Xen version 4.12.0 (@) (gcc (Alpine 6.4.0) 6.4.0) debug=n  Tue Jul 23 
17:15:48 UTC 2019
(XEN) Latest ChangeSet:
(XEN) Bootloader: GRUB 2.03
(XEN) Command line: com1=115200,8n1 console=com1,vga iommu_inclusive_mapping=1 
iommu=debug dom0_mem=1024M,max:1024M dom0_max_vcpus=1 dom0_vcpus_pin smt=false
(XEN) Xen image load base address: 0x8800
(XEN) Video information:
(XEN)  VGA is graphics mode 1680x1050, 32 bpp
(XEN) Disc information:
(XEN)  Found 0 MBR signatures
(XEN)  Found 1 EDD information structures
(XEN) EFI RAM map:
(XEN)   - 00058000 (usable)
(XEN)  00058000 - 00059000 (reserved)
(XEN)  00059000 - 0009f000 (usable)
(XEN)  0009f000 - 000a (reserved)
(XEN)  0010 - 8648a000 (usable)
(XEN)  8648a000 - 8648b000 (ACPI NVS)
(XEN)  8648b000 - 864b5000 (reserved)
(XEN)  864b5000 - 8c224000 (usable)
(XEN)  8c224000 - 8c528000 (reserved)
(XEN)  8c528000 - 8c736000 (usable)
(XEN)  8c736000 - 8cea7000 (ACPI NVS)
(XEN)  8cea7000 - 8d2ff000 (reserved)
(XEN)  8d2ff000 - 8d30 (usable)
(XEN)  8d30 - 8d40 (reserved)
(XEN)  e000 - f000 (reserved)
(XEN)  fe00 - fe011000 (reserved)
(XEN)  fec0 - fec01000 (reserved)
(XEN)  fee0 - fee01000 (reserved)
(XEN)  ff00 - 0001 (reserved)
(XEN)  0001 - 00016e00 (usable)
(XEN) ACPI: RSDP 8CE49000, 0024 (r2 ALASKA)
(XEN) ACPI: XSDT 8CE490A8, 00CC (r1 ALASKA   A M I   1072009 AMI 10013)
(XEN) ACPI: FACP 8CE6C370, 010C (r5 ALASKA   A M I   1072009 AMI 10013)
(XEN) ACPI: DSDT 8CE49208, 23167 (r2 ALASKA   A M I   1072009 INTL 20120913)
(XEN) ACPI: FACS 8CE8EF80, 0040
(XEN) ACPI: APIC 8CE6C480, 0084 (r3 ALASKA   A M I   1072009 AMI 10013)
(XEN) ACPI: FPDT 8CE6C508, 0044 (r1 ALASKA   A M I   1072009 AMI 10013)
(XEN) ACPI: FIDT 8CE6C550, 009C (r1 ALASKA   A M I   1072009 AMI 10013)
(XEN) ACPI: MCFG 8CE6C5F0, 003C (r1 ALASKA   A M I   1072009 MSFT   97)
(XEN) ACPI: HPET 8CE6C630, 0038 (r1 ALASKA   A M I   1072009 AMI.5000B)
(XEN) ACPI: LP

Re: [Xen-devel] [PATCH v2 1/2] xen/x86: cleanup unused NMI/MCE code

[Andrew Cooper]

On 23/07/2019 19:25, Juergen Gross wrote:
> pv_raise_interrupt() is only called for NMIs these days, so the MCE
> specific part can be removed. Rename pv_raise_interrupt() to
> pv_raise_nmi() and NMI_MCE_SOFTIRQ to NMI_SOFTIRQ.

For posterity, it would be helpful to explicitly identify 355b0469a8
which introduced NMI and MCE pinning (where previously there was no NMI
pinning beforehand AFAICT), and then 3a91769d6e which removed the MCE
pinning.

Stated like that, I doubt the NMI pinning was ever relevant in practice.

>
> Additionally there is no need to pin the vcpu the NMI is delivered
> to, that is a leftover of (already removed) MCE handling. So remove
> the pinning, too.
>
> Signed-off-by: Juergen Gross 

Everything LGTM.  A few trivial notes.

> diff --git a/xen/arch/x86/pv/traps.c b/xen/arch/x86/pv/traps.c
> index 1740784ff2..9436c80047 100644
> --- a/xen/arch/x86/pv/traps.c
> +++ b/xen/arch/x86/pv/traps.c
> @@ -136,47 +136,21 @@ bool set_guest_nmi_trapbounce(void)
>  return !null_trap_bounce(curr, tb);
>  }
>  
> -struct softirq_trap {
> -struct domain *domain;   /* domain to inject trap */
> -struct vcpu *vcpu;   /* vcpu to inject trap */
> -unsigned int processor;  /* physical cpu to inject trap */
> -};
> +static DEFINE_PER_CPU(struct vcpu *, softirq_nmi_vcpu);
>  
> -static DEFINE_PER_CPU(struct softirq_trap, softirq_trap);
> -
> -static void nmi_mce_softirq(void)
> +static void nmi_softirq(void)
>  {
>  unsigned int cpu = smp_processor_id();
> -struct softirq_trap *st = &per_cpu(softirq_trap, cpu);
> -
> -BUG_ON(st->vcpu == NULL);
> -
> -/*
> - * Set the tmp value unconditionally, so that the check in the iret
> - * hypercall works.
> - */
> -cpumask_copy(st->vcpu->cpu_hard_affinity_tmp,
> - st->vcpu->cpu_hard_affinity);
> +struct vcpu **v_ptr = &per_cpu(softirq_nmi_vcpu, cpu);

There is only a single use of 'cpu' here, so you can drop that and use
this_cpu(softirq_nmi_vcpu) instead.

> diff --git a/xen/include/asm-x86/pv/traps.h b/xen/include/asm-x86/pv/traps.h
> index fcc75f5e9a..47d6cf5fc4 100644
> --- a/xen/include/asm-x86/pv/traps.h
> +++ b/xen/include/asm-x86/pv/traps.h
> @@ -27,8 +27,8 @@
>  
>  void pv_trap_init(void);
>  
> -/* Deliver interrupt to PV guest. Return 0 on success. */
> -int pv_raise_interrupt(struct vcpu *v, uint8_t vector);
> +/* Deliver NMI to PV guest. Return 0 on success. */
> +int pv_raise_nmi(struct vcpu *v);
>  
>  int pv_emulate_privileged_op(struct cpu_user_regs *regs);
>  void pv_emulate_gate_op(struct cpu_user_regs *regs);
> @@ -46,8 +46,8 @@ static inline bool pv_trap_callback_registered(const struct 
> vcpu *v,
>  
>  static inline void pv_trap_init(void) {}
>  
> -/* Deliver interrupt to PV guest. Return 0 on success. */
> -static inline int pv_raise_interrupt(struct vcpu *v, uint8_t vector) { 
> return -EOPNOTSUPP; }
> +/* Deliver NMI to PV guest. Return 0 on success. */
> +static inline int pv_raise_nmi(struct vcpu *v) { return -EOPNOTSUPP; }

I don't think duplicating the function description here is useful. 
Instead, I'd recommend dropping these lines, and commenting it once in
pv/traps.c.  That should include the fact that it is expected to be used
NMI context, which means its not safe to use printk() etc in there.

~Andrew

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

Re: [Xen-devel] [PATCH v2 2/2] xen: merge temporary vcpu pinning scenarios

[Andrew Cooper]

On 23/07/2019 19:25, Juergen Gross wrote:
> diff --git a/xen/common/schedule.c b/xen/common/schedule.c
> index 349f9624f5..508176a142 100644
> --- a/xen/common/schedule.c
> +++ b/xen/common/schedule.c
> @@ -1106,43 +1106,59 @@ void watchdog_domain_destroy(struct domain *d)
>  kill_timer(&d->watchdog_timer[i]);
>  }
>  
> -int vcpu_pin_override(struct vcpu *v, int cpu)
> +/*
> + * Pin a vcpu temporarily to a specific CPU (or restore old pinning state if
> + * cpu is NR_CPUS).
> + * Temporary pinning can be done due to two reasons, which may be nested:
> + * - VCPU_AFFINITY_OVERRIDE (requested by guest): is allowed to fail in case
> + *   of a conflict (e.g. in case cpupool doesn't include requested CPU, or
> + *   another conflicting temporary pinning is already in effect.
> + * - VCPU_AFFINITY_WAIT (called by wait_event(): only used to pin vcpu to the

Need an extra )

Otherwise, Reviewed-by: Andrew Cooper 

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

Re: [Xen-devel] [PATCH] docs/sphinx: todo/wishlist

[Andrew Cooper]

On 23/07/2019 16:30, Juergen Gross wrote:
> On 23.07.19 15:38, Andrew Cooper wrote:
>> On 23/07/2019 05:36, Juergen Gross wrote:
>>> On 22.07.19 21:20, Andrew Cooper wrote:
 diff --git a/docs/misc/wishlist.rst b/docs/misc/wishlist.rst
 new file mode 100644
 index 00..6cdb47d6e7
 --- /dev/null
 +++ b/docs/misc/wishlist.rst
 @@ -0,0 +1,53 @@
 +Development Wishlist
 +
 +
 +Remove xenstored's dependencies on unstable interfaces
 +--
 +
 +Various xenstored implementations use libxc for two purposes.  It
 would be a
 +substantial advantage to move xenstored onto entirely stable
 interfaces, which
 +disconnects it from the internal of the libxc.
 +
 +1. Foreign mapping of the store ring
 +
 +
 +This is obsolete since :xen-cs:`6a2de353a9` (2012) which allocated
 grant
 +entries instead, to allow xenstored to function as a stub-domain
 without dom0
 +permissions.  :xen-cs:`38eeb3864d` dropped foreign mapping for
 cxenstored.
 +However, there are no OCaml bindings for libxengnttab.
 +
 +Work Items:
 +
 +* Minimal ``tools/ocaml/libs/xg/`` binding for
 ``tools/libs/gnttab/``.
 +* Replicate :xen-cs:`38eeb3864d` for oxenstored as well.
 +
 +
 +2. Figuring out which domain(s) have gone away
 +~~
 +
 +Currently, the handling of domains is asymmetric.
 +
 +* When a domain is created, the toolstack explicitly sends an
 +  ``XS_INTRODUCE(domid, store mfn, store evtchn)`` message to
 xenstored, to
 +  cause xenstored to connect to the guest ring, and fire the
 +  ``@introduceDomain`` watch.
 +* When a domain is destroyed, Xen fires ``VIRQ_DOM_EXC`` which is
 bound by
 +  xenstored, rather than the toolstack.  xenstored updates its idea
 of the
 +  status of domains, and fires the ``@releaseDomain`` watch.
 +
 +Xenstored uses ``xc_domain_getinfo()``, to work out which domain(s)
 have gone
 +away, and only cares about the shutdown status.
 +
 +Furthermore, ``@releaseDomain`` (like ``VIRQ_DOM_EXC``) is a
 single-bit
 +message, which requires all listeners to evaluate whether the
 message applies
 +to them or not.  This results in a flurry of ``xc_domain_getinfo()``
 calls
 +from multiple entities in the system, which all serialise on the
 domctl lock
 +in Xen.
 +
 +Work Items:
 +
 +* Figure out how shutdown status can be expressed in a stable way
 from Xen.
 +* Figure out if ``VIRQ_DOM_EXC`` and ``@releaseDomain`` can be
 extended to
 +  carry at least a domid, to make domain shutdown scale better.
>>>
>>> @releaseDomain (and @introduceDomain) can't be extended, we'd need to
>>> add another watch path like @domainStatus//. Xenstored
>>> could advertise its capability to raise this watch in /tool/xenstored.
>>
>> I guess I was being a bit fast and loose with terminology.  I didn't
>> intend to imply "literally modify @{introduce,release}Domain", as they
>> are already fixed ABIs, but more to "compatibly build something which is
>> better".
>
> Okay.
>
>>
>> That scheme would work for improved @releaseDomain, but it wouldn't work
>> for an improved introduce.  Introduce needs a single key to watch on,
>> which hands back the domid so you don't need to go searching for it.
>
> Yes, and? Its perfectly fine to set a watch firing if anything below
> @domainStatus is changing.

Hmm - that might work if no other information was put into domainStatus,
but would quickly become a scalability problem otherwise.

>
>>
>>>
>>> As VIRQ_DOM_EXC is just an event I don't see how the domid could be
>>> passed by it. I guess we'd need e.g. a shared memory area which the
>>> domain registered for VIRQ_DOM_EXC could map and which would contain a
>>> bitmap (one bit per domain). The hypervisor would set the bit on a
>>> status change and fire VIRQ_DOM_EXC, xenstored would look for a set
>>> bit, clear it and read the status of the related domain.
>>
>> The point here is to avoid using xc_domain_getinfo() in the first place,
>> so there needs to be no "getting the status of the domain".
>
> I'd guess a single xc_domain_getinfo() in the tools wouldn't be so
> problematic. The caller would know the domid already, so no need to
> query all domains.

Its still a problem when you've got 1000 Qemu's, they all get
@releaseDomain, and try to figure out if it is their own domain which
went away.

This shouldn't require taking the domctl lock in Xen 1000 times to
figure out, seeing as xenstored knows exactly which domain actually went
away.

>
>> DOM_EXC is fired for domain_shutdown() only (but for reasons which
>> escape me, fired twice).  Given that a domid is a 15 bit number, a
>> bitmap of all domains does fit with

[Xen-devel] [PATCH] x86/pv: Move async_exception_cleanup() into pv/iret.c

[Andrew Cooper]

All callers are in pv/iret.c.  Move the function and make it static.

Even before the pinning cleanup, there was nothing which is specific to
operating on curr, so rename the variable back to v.

No functional change.

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

Discovered while reviewing/prodding Juergen's pinning removal patches.
---
 xen/arch/x86/pv/iret.c  | 25 -
 xen/arch/x86/traps.c| 24 
 xen/include/asm-x86/traps.h |  2 --
 3 files changed, 24 insertions(+), 27 deletions(-)

diff --git a/xen/arch/x86/pv/iret.c b/xen/arch/x86/pv/iret.c
index c359a1dbfd..ae1c33612b 100644
--- a/xen/arch/x86/pv/iret.c
+++ b/xen/arch/x86/pv/iret.c
@@ -22,7 +22,30 @@
 #include 
 
 #include 
-#include 
+
+static void async_exception_cleanup(struct vcpu *v)
+{
+unsigned int trap;
+
+if ( !v->async_exception_mask )
+return;
+
+if ( !(v->async_exception_mask & (v->async_exception_mask - 1)) )
+trap = __scanbit(v->async_exception_mask, VCPU_TRAP_NONE);
+else
+for ( trap = VCPU_TRAP_NONE + 1; trap <= VCPU_TRAP_LAST; ++trap )
+if ( (v->async_exception_mask ^
+  v->async_exception_state(trap).old_mask) == (1u << trap) )
+break;
+if ( unlikely(trap > VCPU_TRAP_LAST) )
+{
+ASSERT_UNREACHABLE();
+return;
+}
+
+/* Restore previous asynchronous exception mask. */
+v->async_exception_mask = v->async_exception_state(trap).old_mask;
+}
 
 unsigned long do_iret(void)
 {
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 08d7edc568..38d12013db 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1593,30 +1593,6 @@ static void pci_serr_softirq(void)
 outb(inb(0x61) & 0x0b, 0x61); /* re-enable the PCI SERR error line. */
 }
 
-void async_exception_cleanup(struct vcpu *curr)
-{
-int trap;
-
-if ( !curr->async_exception_mask )
-return;
-
-if ( !(curr->async_exception_mask & (curr->async_exception_mask - 1)) )
-trap = __scanbit(curr->async_exception_mask, VCPU_TRAP_NONE);
-else
-for ( trap = VCPU_TRAP_NONE + 1; trap <= VCPU_TRAP_LAST; ++trap )
-if ( (curr->async_exception_mask ^
-  curr->async_exception_state(trap).old_mask) == (1 << trap) )
-break;
-if ( unlikely(trap > VCPU_TRAP_LAST) )
-{
-ASSERT_UNREACHABLE();
-return;
-}
-
-/* Restore previous asynchronous exception mask. */
-curr->async_exception_mask = curr->async_exception_state(trap).old_mask;
-}
-
 static void nmi_hwdom_report(unsigned int reason_idx)
 {
 struct domain *d = hardware_domain;
diff --git a/xen/include/asm-x86/traps.h b/xen/include/asm-x86/traps.h
index b88f2a4f2f..ec23d3a70b 100644
--- a/xen/include/asm-x86/traps.h
+++ b/xen/include/asm-x86/traps.h
@@ -19,8 +19,6 @@
 #ifndef ASM_TRAP_H
 #define ASM_TRAP_H
 
-void async_exception_cleanup(struct vcpu *);
-
 const char *trapstr(unsigned int trapnr);
 
 #endif /* ASM_TRAP_H */
-- 
2.11.0


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

Re: [Xen-devel] [PATCH] libxl: fix pci device re-assigning after domain reboot

[Pasi Kärkkäinen]

Hello,

On Wed, Jul 10, 2019 at 07:44:08PM +0100, Ian Jackson wrote:
> Roger Pau Monne writes ("Re: [Xen-devel] [PATCH] libxl: fix pci device 
> re-assigning after domain reboot"):
> > On Wed, Jun 26, 2019 at 03:37:26PM +0200, Juergen Gross wrote:
> > > Signed-off-by: Juergen Gross 
> > > Tested-by: Chao Gao 
> > 
> > Reviewed-by: Roger Pau Monné 
> 
> Committed, thanks.
> 

I'd like to request backport of this commit to 4.12 branch.

Thanks,

-- Pasi

> Ian.
> 


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

Re: [Xen-devel] preparations for 4.12.1

[Pasi Kärkkäinen]

Hi,

On Fri, Jul 19, 2019 at 02:23:44PM +, Jan Beulich wrote:
> All,
> 
> the release is due in early August. Please point out backports you
> find missing from the respective staging branch, but which you
> consider relevant. The one commit I've queued already on top of
> what was just pushed is
> 
> ec2ab491b5x86/ept: pass correct level to p2m_entry_modify
> 

I'd like to request backport of the following commit for 4.12.1:

"libxl: fix pci device re-assigning after domain reboot":
commit  c19434d9284e93e6f9aaec9a70f5f361adbfaba6

https://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=c19434d9284e93e6f9aaec9a70f5f361adbfaba6


> Jan
>

Thanks,

-- Pasi


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

[Xen-devel] [linux-4.19 test] 139268: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-armhf-pvops 6 kernel-build fail REGR. vs. 129313

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-xl-pvshim 20 guest-start/debian.repeat fail in 139240 pass in 
139268
 test-amd64-i386-qemut-rhel6hvm-amd 12 guest-start/redhat.repeat fail pass in 
139240
 test-arm64-arm64-examine 11 examine-serial/bootloader  fail pass in 139240

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-credit2   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-cubietruck  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-rtds  1 build-check(1)   blocked  n/a
 test-armhf-armhf-examine  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-credit1   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-arndale   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-vhd   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-xsm 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  13 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-arm64-arm64-xl-seattle  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  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-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-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-thunderx 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 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-xl-credit1  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  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-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-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail never pass
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass
 test-amd64-i386-xl-qemut-win7-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-ws16-amd64 17 guest-stop fail never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass
 test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass

version targeted for testing:
 linuxbe9b6782a9eb128a45b4d4fce556f7053234773d
baseline version:
 linux84df9525b0c27f3ebc2ebb1864fa62a97fdedb7d

Last test of basis   129313  2018-11-02 05:39:08 Z  263 days
Failing since129412  2018-11-04 14:10:15 Z  261 days  169 attempts
Testing same since   139240  2019-07-21 22:39:04 Z1 days2 attempts


2285 people touched revisions under test,
not listing them all

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  pass
 build-i386-xsm

Re: [Xen-devel] preparations for 4.12.1

[Roman Shaposhnik]

On Tue, Jul 23, 2019 at 2:00 PM Pasi Kärkkäinen  wrote:
>
> Hi,
>
> On Fri, Jul 19, 2019 at 02:23:44PM +, Jan Beulich wrote:
> > All,
> >
> > the release is due in early August. Please point out backports you
> > find missing from the respective staging branch, but which you
> > consider relevant. The one commit I've queued already on top of
> > what was just pushed is
> >
> > ec2ab491b5x86/ept: pass correct level to p2m_entry_modify
> >
>
> I'd like to request backport of the following commit for 4.12.1:
>
> "libxl: fix pci device re-assigning after domain reboot":
> commit  c19434d9284e93e6f9aaec9a70f5f361adbfaba6
>
> https://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=c19434d9284e93e6f9aaec9a70f5f361adbfaba6

FWIW: I'd like to second that.

Thanks,
Roman.

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

[Xen-devel] [PATCH 3/7] xen/arm: Rework psr_mode_is_32bit()

[Julien Grall]

psr_mode_is_32bit() prototype does not match the rest of the helpers for
the process state. Looking at the callers, most of them will access
struct cpu_user_regs just for calling psr_mode_is_32bit().

The macro is now reworked to take a struct cpu_user_regs in parameter.
At the same time take the opportunity to switch to a static inline
helper.

Lastly, when compiled for 32-bit, Xen will only support 32-bit guest. So
it is pointless to check whether the register state correspond to 64-bit
or not.

Signed-off-by: Julien Grall 
---
 xen/arch/arm/traps.c   | 28 ++--
 xen/include/asm-arm/regs.h |  9 -
 2 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 111a2029e6..54e66a86d0 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -919,7 +919,7 @@ static void _show_registers(const struct cpu_user_regs 
*regs,
 #ifdef CONFIG_ARM_64
 else if ( is_64bit_domain(v->domain) )
 {
-if ( psr_mode_is_32bit(regs->cpsr) )
+if ( psr_mode_is_32bit(regs) )
 {
 BUG_ON(!usr_mode(regs));
 show_registers_32(regs, ctxt, guest_mode, v);
@@ -1625,7 +1625,7 @@ int check_conditional_instr(struct cpu_user_regs *regs, 
const union hsr hsr)
 {
 unsigned long it;
 
-BUG_ON( !psr_mode_is_32bit(regs->cpsr) || !(cpsr&PSR_THUMB) );
+BUG_ON( !psr_mode_is_32bit(regs) || !(cpsr & PSR_THUMB) );
 
 it = ( (cpsr >> (10-2)) & 0xfc) | ((cpsr >> 25) & 0x3 );
 
@@ -1650,7 +1650,7 @@ int check_conditional_instr(struct cpu_user_regs *regs, 
const union hsr hsr)
 void advance_pc(struct cpu_user_regs *regs, const union hsr hsr)
 {
 unsigned long itbits, cond, cpsr = regs->cpsr;
-bool is_thumb = psr_mode_is_32bit(cpsr) && (cpsr & PSR_THUMB);
+bool is_thumb = psr_mode_is_32bit(regs) && (cpsr & PSR_THUMB);
 
 if ( is_thumb && (cpsr & PSR_IT_MASK) )
 {
@@ -2078,32 +2078,32 @@ void do_trap_guest_sync(struct cpu_user_regs *regs)
 advance_pc(regs, hsr);
 break;
 case HSR_EC_CP15_32:
-GUEST_BUG_ON(!psr_mode_is_32bit(regs->cpsr));
+GUEST_BUG_ON(!psr_mode_is_32bit(regs));
 perfc_incr(trap_cp15_32);
 do_cp15_32(regs, hsr);
 break;
 case HSR_EC_CP15_64:
-GUEST_BUG_ON(!psr_mode_is_32bit(regs->cpsr));
+GUEST_BUG_ON(!psr_mode_is_32bit(regs));
 perfc_incr(trap_cp15_64);
 do_cp15_64(regs, hsr);
 break;
 case HSR_EC_CP14_32:
-GUEST_BUG_ON(!psr_mode_is_32bit(regs->cpsr));
+GUEST_BUG_ON(!psr_mode_is_32bit(regs));
 perfc_incr(trap_cp14_32);
 do_cp14_32(regs, hsr);
 break;
 case HSR_EC_CP14_64:
-GUEST_BUG_ON(!psr_mode_is_32bit(regs->cpsr));
+GUEST_BUG_ON(!psr_mode_is_32bit(regs));
 perfc_incr(trap_cp14_64);
 do_cp14_64(regs, hsr);
 break;
 case HSR_EC_CP14_DBG:
-GUEST_BUG_ON(!psr_mode_is_32bit(regs->cpsr));
+GUEST_BUG_ON(!psr_mode_is_32bit(regs));
 perfc_incr(trap_cp14_dbg);
 do_cp14_dbg(regs, hsr);
 break;
 case HSR_EC_CP:
-GUEST_BUG_ON(!psr_mode_is_32bit(regs->cpsr));
+GUEST_BUG_ON(!psr_mode_is_32bit(regs));
 perfc_incr(trap_cp);
 do_cp(regs, hsr);
 break;
@@ -2114,7 +2114,7 @@ void do_trap_guest_sync(struct cpu_user_regs *regs)
  * ARMv7 (DDI 0406C.b): B1.14.8
  * ARMv8 (DDI 0487A.d): D1-1501 Table D1-44
  */
-GUEST_BUG_ON(!psr_mode_is_32bit(regs->cpsr));
+GUEST_BUG_ON(!psr_mode_is_32bit(regs));
 perfc_incr(trap_smc32);
 do_trap_smc(regs, hsr);
 break;
@@ -2122,7 +2122,7 @@ void do_trap_guest_sync(struct cpu_user_regs *regs)
 {
 register_t nr;
 
-GUEST_BUG_ON(!psr_mode_is_32bit(regs->cpsr));
+GUEST_BUG_ON(!psr_mode_is_32bit(regs));
 perfc_incr(trap_hvc32);
 #ifndef NDEBUG
 if ( (hsr.iss & 0xff00) == 0xff00 )
@@ -2137,7 +2137,7 @@ void do_trap_guest_sync(struct cpu_user_regs *regs)
 }
 #ifdef CONFIG_ARM_64
 case HSR_EC_HVC64:
-GUEST_BUG_ON(psr_mode_is_32bit(regs->cpsr));
+GUEST_BUG_ON(psr_mode_is_32bit(regs));
 perfc_incr(trap_hvc64);
 #ifndef NDEBUG
 if ( (hsr.iss & 0xff00) == 0xff00 )
@@ -2153,12 +2153,12 @@ void do_trap_guest_sync(struct cpu_user_regs *regs)
  *
  * ARMv8 (DDI 0487A.d): D1-1501 Table D1-44
  */
-GUEST_BUG_ON(psr_mode_is_32bit(regs->cpsr));
+GUEST_BUG_ON(psr_mode_is_32bit(regs));
 perfc_incr(trap_smc64);
 do_trap_smc(regs, hsr);
 break;
 case HSR_EC_SYSREG:
-GUEST_BUG_ON(psr_mode_is_32bit(regs->cpsr));
+GUEST_BUG_ON(psr_mode_is_32bit(regs));
 perfc_incr(trap_sysreg);
 do_sysreg(regs, hsr);
 break;
diff --git a/xen/include/asm-arm/regs.h b/xen/include/asm-arm/regs.h
index ddc6eba9ce..0e3e5

[Xen-devel] [PATCH 1/7] xen/public: arch-arm: Restrict the visibility of struct vcpu_guest_core_regs

[Julien Grall]

Currently, the structure vcpu_guest_core_regs is part of the public API.
This implies that any change in the structure should be backward
compatible.

However, the structure is only needed by the tools and Xen. It is also
not expected to be ever used outside of that context. So we could save us
some headache by only declaring the structure for Xen and tools.

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

---
This is a follow-up of the discussion [1].

[1] <3c245c5b-51c6-1d0e-ad6c-424145731...@arm.com>

Changes in v3:
- Avoid introduce a new #ifdef in the header by moving the
definitions later on.
---
 xen/include/public/arch-arm.h | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index 3e8cdc151d..7ce139a0f5 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -197,6 +197,18 @@
 } while ( 0 )
 #define set_xen_guest_handle(hnd, val) set_xen_guest_handle_raw(hnd, val)
 
+typedef uint64_t xen_pfn_t;
+#define PRI_xen_pfn PRIx64
+#define PRIu_xen_pfn PRIu64
+
+/* Maximum number of virtual CPUs in legacy multi-processor guests. */
+/* Only one. All other VCPUS must use VCPUOP_register_vcpu_info */
+#define XEN_LEGACY_MAX_VCPUS 1
+
+typedef uint64_t xen_ulong_t;
+#define PRI_xen_ulong PRIx64
+
+#if defined(__XEN__) || defined(__XEN_TOOLS__)
 #if defined(__GNUC__) && !defined(__STRICT_ANSI__)
 /* Anonymous union includes both 32- and 64-bit names (e.g., r0/x0). */
 # define __DECL_REG(n64, n32) union {  \
@@ -272,18 +284,6 @@ DEFINE_XEN_GUEST_HANDLE(vcpu_guest_core_regs_t);
 
 #undef __DECL_REG
 
-typedef uint64_t xen_pfn_t;
-#define PRI_xen_pfn PRIx64
-#define PRIu_xen_pfn PRIu64
-
-/* Maximum number of virtual CPUs in legacy multi-processor guests. */
-/* Only one. All other VCPUS must use VCPUOP_register_vcpu_info */
-#define XEN_LEGACY_MAX_VCPUS 1
-
-typedef uint64_t xen_ulong_t;
-#define PRI_xen_ulong PRIx64
-
-#if defined(__XEN__) || defined(__XEN_TOOLS__)
 struct vcpu_guest_context {
 #define _VGCF_online   0
 #define VGCF_online(1<<_VGCF_online)
-- 
2.11.0


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

[Xen-devel] [PATCH 6/7] xen/arm: vsmc: The function identifier is always 32-bit

[Julien Grall]

On Arm64, the SMCCC function identifier is always stored in the first 32-bit
of x0 register. The rest of the bits are not defined and should be
ignored.

This means the variable funcid should be an uint32_t rather than
register_t.

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

diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
index f8e350311d..a36db15fff 100644
--- a/xen/arch/arm/vsmc.c
+++ b/xen/arch/arm/vsmc.c
@@ -220,7 +220,7 @@ static bool vsmccc_handle_call(struct cpu_user_regs *regs)
 {
 bool handled = false;
 const union hsr hsr = { .bits = regs->hsr };
-register_t funcid = get_user_reg(regs, 0);
+uint32_t funcid = get_user_reg(regs, 0);
 
 /*
  * Check immediate value for HVC32, HVC64 and SMC64.
@@ -286,7 +286,7 @@ static bool vsmccc_handle_call(struct cpu_user_regs *regs)
 
 if ( !handled )
 {
-gprintk(XENLOG_INFO, "Unhandled SMC/HVC: %08"PRIregister"\n", funcid);
+gprintk(XENLOG_INFO, "Unhandled SMC/HVC: %#x\n", funcid);
 
 /* Inform caller that function is not supported. */
 set_user_reg(regs, 0, ARM_SMCCC_ERR_UNKNOWN_FUNCTION);
-- 
2.11.0


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

[Xen-devel] [PATCH 0/7] xen/arm: Xen hardening for newer Armv8

[Julien Grall]

Hi all,

This is a not-yet complete series to harden Xen for later revision of
Armv8. The main goals are:
- Reducing the number of BUG_ON() to check guest state
- Fix system registers size as they are always 64-bit on AArch64
(not 32-bit!).

There are more work to do. I will send them in smaller batch as I find
spare time to rework bits of Xen.

Note that patch #1 was already sent separately but added here for convenience.

Cheers,

Julien Grall (7):
  xen/public: arch-arm: Restrict the visibility of struct
vcpu_guest_core_regs
  xen/arm: SCTLR_EL1 is a 64-bit register on Arm64
  xen/arm: Rework psr_mode_is_32bit()
  xen/arm: traps: Avoid using BUG_ON() in _show_registers()
  xen/arm: traps: Avoid BUG_ON() in do_trap_brk()
  xen/arm: vsmc: The function identifier is always 32-bit
  xen/arm: types: Specify the zero padding in the definition of
PRIregister

 tools/xentrace/xenctx.c   |  4 ++-
 xen/arch/arm/guest_walk.c |  2 +-
 xen/arch/arm/traps.c  | 73 ---
 xen/arch/arm/vsmc.c   |  4 +--
 xen/include/asm-arm/domain.h  |  3 +-
 xen/include/asm-arm/p2m.h |  4 +--
 xen/include/asm-arm/regs.h|  9 +-
 xen/include/asm-arm/types.h   |  4 +--
 xen/include/public/arch-arm.h | 28 -
 9 files changed, 68 insertions(+), 63 deletions(-)

-- 
2.11.0


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

[Xen-devel] [PATCH 5/7] xen/arm: traps: Avoid BUG_ON() in do_trap_brk()

[Julien Grall]

At the moment, do_trap_brk() is using a BUG_ON() to check the hardware
has been correctly configured during boot.

Any error when configuring the hardware could result to a guest 'brk'
trapping in the hypervisor and crash it.

This is pretty harsh to kill Xen when actually killing the guest would
be enough as misconfiguring this trap would not lead to exposing
sensitive data. Replace the BUG_ON() with crashing the guest.

Signed-off-by: Julien Grall 
---
 xen/arch/arm/traps.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 132686ee0f..ef37ca6bde 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1304,10 +1304,15 @@ int do_bug_frame(const struct cpu_user_regs *regs, 
vaddr_t pc)
 #ifdef CONFIG_ARM_64
 static void do_trap_brk(struct cpu_user_regs *regs, const union hsr hsr)
 {
-/* HCR_EL2.TGE and MDCR_EL2.TDE are not set so we never receive
- * software breakpoint exception for EL1 and EL0 here.
+/*
+ * HCR_EL2.TGE and MDCR_EL2.TDR are currently not set. So we should
+ * never receive software breakpoing exception for EL1 and EL0 here.
  */
-BUG_ON(!hyp_mode(regs));
+if ( !hyp_mode(regs) )
+{
+domain_crash(current->domain);
+return;
+}
 
 switch ( hsr.brk.comment )
 {
-- 
2.11.0


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

[Xen-devel] [PATCH 7/7] xen/arm: types: Specify the zero padding in the definition of PRIregister

[Julien Grall]

The definition of PRIregister varies between Arm32 and Arm64 (32-bit vs
64-bit). However, some of the users uses the wrong padding.

For more consistency, the padding is now moved into the PRIregister and
varies depending on the architecture.

Signed-off-by: Julien Grall 
---
 xen/arch/arm/traps.c| 10 +-
 xen/include/asm-arm/types.h |  4 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index ef37ca6bde..f062ae6f6a 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -797,7 +797,7 @@ static void show_registers_32(const struct cpu_user_regs 
*regs,
 
 if ( guest_mode )
 {
-printk("USR: SP: %08"PRIx32" LR: %08"PRIregister"\n",
+printk("USR: SP: %08"PRIx32" LR: %"PRIregister"\n",
regs->sp_usr, regs->lr);
 printk("SVC: SP: %08"PRIx32" LR: %08"PRIx32" SPSR:%08"PRIx32"\n",
regs->sp_svc, regs->lr_svc, regs->spsr_svc);
@@ -815,7 +815,7 @@ static void show_registers_32(const struct cpu_user_regs 
*regs,
 #ifndef CONFIG_ARM_64
 else
 {
-printk("HYP: SP: %08"PRIx32" LR: %08"PRIregister"\n", regs->sp, 
regs->lr);
+printk("HYP: SP: %08"PRIx32" LR: %"PRIregister"\n", regs->sp, 
regs->lr);
 }
 #endif
 printk("\n");
@@ -823,7 +823,7 @@ static void show_registers_32(const struct cpu_user_regs 
*regs,
 if ( guest_mode )
 {
 printk(" SCTLR: %"PRIregister"\n", ctxt->sctlr_el1);
-printk("   TCR: %08"PRIregister"\n", ctxt->tcr_el1);
+printk("   TCR: %"PRIregister"\n", ctxt->tcr_el1);
 printk(" TTBR0: %016"PRIx64"\n", ctxt->ttbr0_el1);
 printk(" TTBR1: %016"PRIx64"\n", ctxt->ttbr1_el1);
 printk("  IFAR: %08"PRIx32", IFSR: %08"PRIx32"\n"
@@ -895,7 +895,7 @@ static void show_registers_64(const struct cpu_user_regs 
*regs,
 printk("   FAR_EL1: %016"PRIx64"\n", ctxt->far);
 printk("\n");
 printk(" SCTLR_EL1: %"PRIregister"\n", ctxt->sctlr_el1);
-printk("   TCR_EL1: %08"PRIregister"\n", ctxt->tcr_el1);
+printk("   TCR_EL1: %"PRIregister"\n", ctxt->tcr_el1);
 printk(" TTBR0_EL1: %016"PRIx64"\n", ctxt->ttbr0_el1);
 printk(" TTBR1_EL1: %016"PRIx64"\n", ctxt->ttbr1_el1);
 printk("\n");
@@ -934,7 +934,7 @@ static void _show_registers(const struct cpu_user_regs 
*regs,
 printk("\n");
 
 printk(" SCTLR_EL2: %08"PRIx32"\n", READ_SYSREG32(SCTLR_EL2));
-printk("   HCR_EL2: %016"PRIregister"\n", READ_SYSREG(HCR_EL2));
+printk("   HCR_EL2: %"PRIregister"\n", READ_SYSREG(HCR_EL2));
 printk(" TTBR0_EL2: %016"PRIx64"\n", READ_SYSREG64(TTBR0_EL2));
 printk("\n");
 printk("   ESR_EL2: %08"PRIx32"\n", regs->hsr);
diff --git a/xen/include/asm-arm/types.h b/xen/include/asm-arm/types.h
index 30f95078cb..89aae25ffe 100644
--- a/xen/include/asm-arm/types.h
+++ b/xen/include/asm-arm/types.h
@@ -41,7 +41,7 @@ typedef u64 paddr_t;
 #define INVALID_PADDR (~0ULL)
 #define PRIpaddr "016llx"
 typedef u32 register_t;
-#define PRIregister "x"
+#define PRIregister "08x"
 #elif defined (CONFIG_ARM_64)
 typedef signed long s64;
 typedef unsigned long u64;
@@ -51,7 +51,7 @@ typedef u64 paddr_t;
 #define INVALID_PADDR (~0UL)
 #define PRIpaddr "016lx"
 typedef u64 register_t;
-#define PRIregister "lx"
+#define PRIregister "016lx"
 #endif
 
 #if defined(__SIZE_TYPE__)
-- 
2.11.0


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

[Xen-devel] [PATCH 4/7] xen/arm: traps: Avoid using BUG_ON() in _show_registers()

[Julien Grall]

At the moment, _show_registers() is using a BUG_ON() to assert only
userspace will run 32-bit code in a 64-bit domain.

Such extra precaution is not necessary and could be avoided by only
checking the CPU mode to decide whether show_registers_64() or
show_reigsters_32() should be called.

This has also the nice advantage to avoid nested if in the code.

Signed-off-by: Julien Grall 
---
 xen/arch/arm/traps.c | 16 +++-
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 54e66a86d0..132686ee0f 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -914,21 +914,11 @@ static void _show_registers(const struct cpu_user_regs 
*regs,
 
 if ( guest_mode )
 {
-if ( is_32bit_domain(v->domain) )
+if ( psr_mode_is_32bit(regs) )
 show_registers_32(regs, ctxt, guest_mode, v);
 #ifdef CONFIG_ARM_64
-else if ( is_64bit_domain(v->domain) )
-{
-if ( psr_mode_is_32bit(regs) )
-{
-BUG_ON(!usr_mode(regs));
-show_registers_32(regs, ctxt, guest_mode, v);
-}
-else
-{
-show_registers_64(regs, ctxt, guest_mode, v);
-}
-}
+else
+show_registers_64(regs, ctxt, guest_mode, v);
 #endif
 }
 else
-- 
2.11.0


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

[Xen-devel] [PATCH 2/7] xen/arm: SCTLR_EL1 is a 64-bit register on Arm64

[Julien Grall]

On Arm64, system registers are always 64-bit including SCTLR_EL1.
However, Xen is assuming this is 32-bit because earlier revision of
Armv8 had the top 32-bit RES0 (see ARM DDI0595.b).

From Armv8.5, some bits in [63:32] will be defined and allowed to be
modified by the guest. So we would effectively reset those bits to 0
after each context switch. This means the guest may not function
correctly afterwards.

Rather than resetting to 0 the bits [63:32], preserve them acxcross
context switch.

Note that the corresponding register on Arm32 (i.e SCTLR) is always
32-bit. So we need to use register_t anywhere we deal the SCTLR{,_EL1}.

Outside interface is switched to use 64-bit to allow ABI compatibility
between 32-bit and 64-bit.

Signed-off-by: Julien Grall 

---
All the other system registers should be switched to 64-bit. This is
done separatly as this is the only system register that currently
not save/restore correctly.

I would consider to backport it as we would end up to disable
features behind the back of the guest.
---
 tools/xentrace/xenctx.c   |  4 +++-
 xen/arch/arm/guest_walk.c |  2 +-
 xen/arch/arm/traps.c  | 10 +-
 xen/include/asm-arm/domain.h  |  3 ++-
 xen/include/asm-arm/p2m.h |  4 ++--
 xen/include/public/arch-arm.h |  4 ++--
 6 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c
index e647179e19..2fa864f867 100644
--- a/tools/xentrace/xenctx.c
+++ b/tools/xentrace/xenctx.c
@@ -598,6 +598,8 @@ static void print_ctx_32(vcpu_guest_context_t *ctx)
 
 printf("r12_fiq: %08"PRIx32"\n", regs->r12_fiq);
 printf("\n");
+/* SCTLR is always 32-bit */
+printf("SCTLR: %08"PRIx32"\n", (uint32_t)ctx->sctlr);
 }
 
 #ifdef __aarch64__
@@ -659,6 +661,7 @@ static void print_ctx_64(vcpu_guest_context_t *ctx)
 printf("x28: %016"PRIx64"\t", regs->x28);
 printf("x29: %016"PRIx64"\n", regs->x29);
 printf("\n");
+printf("SCTLR_EL1: %016"PRIx64"\n", ctx->sctlr);
 }
 #endif /* __aarch64__ */
 
@@ -675,7 +678,6 @@ static void print_ctx(vcpu_guest_context_any_t *ctx_any)
 print_ctx_32(ctx);
 #endif
 
-printf("SCTLR: %08"PRIx32"\n", ctx->sctlr);
 printf("TTBCR: %016"PRIx64"\n", ctx->ttbcr);
 printf("TTBR0: %016"PRIx64"\n", ctx->ttbr0);
 printf("TTBR1: %016"PRIx64"\n", ctx->ttbr1);
diff --git a/xen/arch/arm/guest_walk.c b/xen/arch/arm/guest_walk.c
index c6d6e23bf5..a1cdd7f4af 100644
--- a/xen/arch/arm/guest_walk.c
+++ b/xen/arch/arm/guest_walk.c
@@ -589,7 +589,7 @@ static bool guest_walk_ld(const struct vcpu *v,
 bool guest_walk_tables(const struct vcpu *v, vaddr_t gva,
paddr_t *ipa, unsigned int *perms)
 {
-uint32_t sctlr = READ_SYSREG(SCTLR_EL1);
+register_t sctlr = READ_SYSREG(SCTLR_EL1);
 register_t tcr = READ_SYSREG(TCR_EL1);
 unsigned int _perms;
 
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 3103620323..111a2029e6 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -384,7 +384,7 @@ void panic_PAR(uint64_t par)
 
 static void cpsr_switch_mode(struct cpu_user_regs *regs, int mode)
 {
-uint32_t sctlr = READ_SYSREG32(SCTLR_EL1);
+register_t sctlr = READ_SYSREG(SCTLR_EL1);
 
 regs->cpsr &= 
~(PSR_MODE_MASK|PSR_IT_MASK|PSR_JAZELLE|PSR_BIG_ENDIAN|PSR_THUMB);
 
@@ -400,7 +400,7 @@ static void cpsr_switch_mode(struct cpu_user_regs *regs, 
int mode)
 
 static vaddr_t exception_handler32(vaddr_t offset)
 {
-uint32_t sctlr = READ_SYSREG32(SCTLR_EL1);
+register_t sctlr = READ_SYSREG(SCTLR_EL1);
 
 if ( sctlr & SCTLR_A32_EL1_V )
 return 0x + offset;
@@ -719,7 +719,7 @@ crash_system:
 
 struct reg_ctxt {
 /* Guest-side state */
-uint32_t sctlr_el1;
+register_t sctlr_el1;
 register_t tcr_el1;
 uint64_t ttbr0_el1, ttbr1_el1;
 #ifdef CONFIG_ARM_32
@@ -822,7 +822,7 @@ static void show_registers_32(const struct cpu_user_regs 
*regs,
 
 if ( guest_mode )
 {
-printk(" SCTLR: %08"PRIx32"\n", ctxt->sctlr_el1);
+printk(" SCTLR: %"PRIregister"\n", ctxt->sctlr_el1);
 printk("   TCR: %08"PRIregister"\n", ctxt->tcr_el1);
 printk(" TTBR0: %016"PRIx64"\n", ctxt->ttbr0_el1);
 printk(" TTBR1: %016"PRIx64"\n", ctxt->ttbr1_el1);
@@ -894,7 +894,7 @@ static void show_registers_64(const struct cpu_user_regs 
*regs,
 printk("   ESR_EL1: %08"PRIx32"\n", ctxt->esr_el1);
 printk("   FAR_EL1: %016"PRIx64"\n", ctxt->far);
 printk("\n");
-printk(" SCTLR_EL1: %08"PRIx32"\n", ctxt->sctlr_el1);
+printk(" SCTLR_EL1: %"PRIregister"\n", ctxt->sctlr_el1);
 printk("   TCR_EL1: %08"PRIregister"\n", ctxt->tcr_el1);
 printk(" TTBR0_EL1: %016"PRIx64"\n", ctxt->ttbr0_el1);
 printk(" TTBR1_EL1: %016"PRIx64"\n", ctxt->ttbr1_el1);
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 2960a53e69..86ebdd2bcf 100644
--- a/xen/include/asm-

Re: [Xen-devel] Criteria / validation proposal: drop Xen

[Adam Williamson]

On Thu, 2019-07-11 at 14:19 -0700, Adam Williamson wrote:
> On Thu, 2019-07-11 at 21:43 +0100, Peter Robinson wrote:
> > > On Mon, 2019-07-08 at 09:11 -0700, Adam Williamson wrote:
> > > > It's worth noting that at least part of the justification for the
> > > > criterion in the first place was that Amazon was using Xen for EC2, but
> > > > that is no longer the case, most if not all EC2 instance types no
> > > > longer use Xen.
> > > 
> > > I don't know where you got that particular piece of information. It
> > > isn't correct. Most EC2 instance types still use Xen. The vast majority
> > > of EC2 instances, by volume, are Xen.
> > 
> > Correct, it's only specific types of new hypervisors that use kvm
> > based, plus new HW like aarch64.
> > 
> > That being said I don't believe testing we can boot on xen is actually
> > useful these days for the AWS use case, it's likely different enough
> > that the testing isn't useful, we'd be much better testing that cloud
> > images actually work on AWS than testing if it boots on xen.
> 
> Yeah, that's where I was going to go next (there has already been a
> thread about this this morning). If what we care about is that Fedora
> boots on EC2, that's what we should have in the criteria, and what we
> should test.
> 
> IIRC, what we have right now is a somewhat vague setup where we just
> have 'local', 'ec2' and 'openstack' columns. The instructions for
> "Amazon Web Services" just say "Launch an instance with the AMI under
> test". So we could probably stand to tighten that up a bit, and define
> specific instance type(s) that we want to test/block on.

OK, so, to move forward with this (and looping in cloud list): does
someone want to propose a set (ideally small - 2 would be great, one
Xen and one non-Xen, if we can cover most common usages that way!) of
EC2 instance types we should test on? With that, we could tweak the
criteria a bit to specify those instance types, tweak the Cloud
validation page a bit, and then drop the Xen criterion and test case.

Thanks everyone!
-- 
Adam Williamson
Fedora QA Community Monkey
IRC: adamw | Twitter: AdamW_Fedora | XMPP: adamw AT happyassassin . net
http://www.happyassassin.net


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