Re: [Xen-devel] [PATCH v4 2/2] xen/kbdif: Add features to disable keyboard and pointer

[Oleksandr Andrushchenko]

On 05/17/2018 08:50 AM, Juergen Gross wrote:

On 17/05/18 07:45, Oleksandr Andrushchenko wrote:

Hi, Juergen!

This patch should go into 4.11 as it is needed for a related Linux
kernel patch and the risk is next to zero for Xen due to only adding
some macros not in use on Xen side.

Could you please release ack this

Release-acked-by: Juergen Gross 

Thank you

and apply?

This has to be done by a committer, which I'm not.

Konrad, could you please apply?


Juergen

Thank you,
Oleksandr

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

Re: [Xen-devel] [PATCH v4 2/2] xen/kbdif: Add features to disable keyboard and pointer

[Juergen Gross]

On 17/05/18 07:45, Oleksandr Andrushchenko wrote:
> Hi, Juergen!
> 
> This patch should go into 4.11 as it is needed for a related Linux
> kernel patch and the risk is next to zero for Xen due to only adding
> some macros not in use on Xen side.
> 
> Could you please release ack this

Release-acked-by: Juergen Gross 

> and apply?

This has to be done by a committer, which I'm not.


Juergen

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

Re: [Xen-devel] [PATCH v4 1/2] xen/kbdif: Add string constants for raw pointer

[Oleksandr Andrushchenko]

On 05/17/2018 08:42 AM, Juergen Gross wrote:

On 17/05/18 07:29, Oleksandr Andrushchenko wrote:

On 05/17/2018 07:28 AM, Juergen Gross wrote:

On 16/05/18 06:59, Oleksandr Andrushchenko wrote:

On 05/11/2018 06:15 PM, Juergen Gross wrote:

On 11/05/18 15:38, Konrad Rzeszutek Wilk wrote:

On Fri, May 11, 2018 at 09:37:57AM -0400, Konrad Rzeszutek Wilk wrote:

On Wed, May 02, 2018 at 05:49:18PM +0300, Oleksandr Andrushchenko
wrote:

From: Oleksandr Andrushchenko 

Add missing string constants for {feature|request}-raw-pointer
to align with the rest of the interface file.

Fixes 7868654ff7fe ("kbdif: Define "feature-raw-pointer" and
"request-raw-pointer")

Signed-off-by: Oleksandr Andrushchenko


Reviewed-by: Konrad Rzeszutek Wilk 

Juergen, you OK with an release-ack?

Yes:

Release-acked-by: Juergen Gross 

Is this ack for both patches or this one only?

It was meant for patch 1 only.

Clear

I see Konrad has applied this patch, but
"[Xen-devel][PATCH v4 2/2] xen/kbdif: Add features to disable keyboard
and pointer"
is still floating.

In principle I'm fine with taking it for 4.11, but I think at this stage
of the release there should be a formal request to include it.

...and the request should come from Konrad?
Or who else may request that?

Normally the sender of the patch would make this request below the ---
of the commit message, e.g.:
   ---
   This patch should go into 4.11 as it is needed for a related Linux
   kernel patch and the risk is next to zero for Xen due to only adding
   some macros not in use on Xen side.

The request can be made by anyone, of course, e.g. by replying to the
patch mail and including the release manager as a recipient.

Great, thank you

And, as I have r-b's from Konrad for both patches,
will it be ok if we apply the corresponding changes to the
kernel now, so my change to xen-kbdfront is unblocked?

No, please don't do that.

Sure


Juergen

Thank you,
Oleksandr

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

Re: [Xen-devel] [PATCH v4 2/2] xen/kbdif: Add features to disable keyboard and pointer

[Oleksandr Andrushchenko]

Hi, Juergen!

This patch should go into 4.11 as it is needed for a related Linux
kernel patch and the risk is next to zero for Xen due to only adding
some macros not in use on Xen side.

Could you please release ack this and apply?

Thank you,
Oleksandr

On 05/02/2018 05:49 PM, Oleksandr Andrushchenko wrote:

From: Oleksandr Andrushchenko 

It is now not fully possible to control if and which virtual devices
are created by the frontend, e.g. keyboard and pointer devices
are always created and multi-touch device is created if the
backend advertises multi-touch support. In some cases this
behavior is not desirable and better control over the frontend's
configuration is required.

Add new XenStore feature fields, so it is possible to individually
control set of exposed virtual devices for each guest OS:
  - set feature-disable-keyboard to 1 if no keyboard device needs
to be created
  - set feature-disable-pointer to 1 if no pointer device needs
to be created

Keep old behavior by default.

Signed-off-by: Oleksandr Andrushchenko 
Reviewed-by: Konrad Rzeszutek Wilk 
---
  xen/include/public/io/kbdif.h | 14 ++
  1 file changed, 14 insertions(+)

diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h
index daf4bc2063c9..23d1f70d5210 100644
--- a/xen/include/public/io/kbdif.h
+++ b/xen/include/public/io/kbdif.h
@@ -51,6 +51,18 @@
   * corresponding entries in XenStore and puts 1 as the value of the entry.
   * If a feature is not supported then 0 must be set or feature entry omitted.
   *
+ * feature-disable-keyboard
+ *  Values: 
+ *
+ *  If there is no need to expose a virtual keyboard device by the
+ *  frontend then this must be set to 1.
+ *
+ * feature-disable-pointer
+ *  Values: 
+ *
+ *  If there is no need to expose a virtual pointer device by the
+ *  frontend then this must be set to 1.
+ *
   * feature-abs-pointer
   *  Values: 
   *
@@ -177,6 +189,8 @@
  
  #define XENKBD_DRIVER_NAME "vkbd"
  
+#define XENKBD_FIELD_FEAT_DSBL_KEYBRD  "feature-disable-keyboard"

+#define XENKBD_FIELD_FEAT_DSBL_POINTER "feature-disable-pointer"
  #define XENKBD_FIELD_FEAT_ABS_POINTER  "feature-abs-pointer"
  #define XENKBD_FIELD_FEAT_RAW_POINTER  "feature-raw-pointer"
  #define XENKBD_FIELD_FEAT_MTOUCH   "feature-multi-touch"



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

Re: [Xen-devel] [PATCH v4 1/2] xen/kbdif: Add string constants for raw pointer

[Juergen Gross]

On 17/05/18 07:29, Oleksandr Andrushchenko wrote:
> On 05/17/2018 07:28 AM, Juergen Gross wrote:
>> On 16/05/18 06:59, Oleksandr Andrushchenko wrote:
>>> On 05/11/2018 06:15 PM, Juergen Gross wrote:
 On 11/05/18 15:38, Konrad Rzeszutek Wilk wrote:
> On Fri, May 11, 2018 at 09:37:57AM -0400, Konrad Rzeszutek Wilk wrote:
>> On Wed, May 02, 2018 at 05:49:18PM +0300, Oleksandr Andrushchenko
>> wrote:
>>> From: Oleksandr Andrushchenko 
>>>
>>> Add missing string constants for {feature|request}-raw-pointer
>>> to align with the rest of the interface file.
>>>
>>> Fixes 7868654ff7fe ("kbdif: Define "feature-raw-pointer" and
>>> "request-raw-pointer")
>>>
>>> Signed-off-by: Oleksandr Andrushchenko
>>> 
>> Reviewed-by: Konrad Rzeszutek Wilk 
> Juergen, you OK with an release-ack?
 Yes:

 Release-acked-by: Juergen Gross 
>>> Is this ack for both patches or this one only?
>> It was meant for patch 1 only.
> Clear
>>> I see Konrad has applied this patch, but
>>> "[Xen-devel][PATCH v4 2/2] xen/kbdif: Add features to disable keyboard
>>> and pointer"
>>> is still floating.
>> In principle I'm fine with taking it for 4.11, but I think at this stage
>> of the release there should be a formal request to include it.
> ...and the request should come from Konrad?
> Or who else may request that?

Normally the sender of the patch would make this request below the ---
of the commit message, e.g.:
  ---
  This patch should go into 4.11 as it is needed for a related Linux
  kernel patch and the risk is next to zero for Xen due to only adding
  some macros not in use on Xen side.

The request can be made by anyone, of course, e.g. by replying to the
patch mail and including the release manager as a recipient.

> And, as I have r-b's from Konrad for both patches,
> will it be ok if we apply the corresponding changes to the
> kernel now, so my change to xen-kbdfront is unblocked?

No, please don't do that.


Juergen

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

Re: [Xen-devel] [PATCH v3 2/2] Input: xen-kbdfront - allow better run-time configuration

[Oleksandr Andrushchenko]

On 05/17/2018 12:08 AM, Dmitry Torokhov wrote:

On Wed, May 16, 2018 at 08:47:30PM +0300, Oleksandr Andrushchenko wrote:

On 05/16/2018 08:15 PM, Dmitry Torokhov wrote:

Hi Oleksandr,

On Mon, May 14, 2018 at 05:40:29PM +0300, Oleksandr Andrushchenko wrote:

@@ -211,93 +220,114 @@ static int xenkbd_probe(struct xenbus_device *dev,
if (!info->page)
goto error_nomem;
-   /* Set input abs params to match backend screen res */
-   abs = xenbus_read_unsigned(dev->otherend,
-  XENKBD_FIELD_FEAT_ABS_POINTER, 0);
-   ptr_size[KPARAM_X] = xenbus_read_unsigned(dev->otherend,
- XENKBD_FIELD_WIDTH,
- ptr_size[KPARAM_X]);
-   ptr_size[KPARAM_Y] = xenbus_read_unsigned(dev->otherend,
- XENKBD_FIELD_HEIGHT,
- ptr_size[KPARAM_Y]);
-   if (abs) {
-   ret = xenbus_write(XBT_NIL, dev->nodename,
-  XENKBD_FIELD_REQ_ABS_POINTER, "1");
-   if (ret) {
-   pr_warn("xenkbd: can't request abs-pointer\n");
-   abs = 0;
-   }
-   }
+   /*
+* The below are reverse logic, e.g. if the feature is set, then
+* do not expose the corresponding virtual device.
+*/
+   with_kbd = !xenbus_read_unsigned(dev->nodename,
+XENKBD_FIELD_FEAT_DSBL_KEYBRD, 0);
-   touch = xenbus_read_unsigned(dev->nodename,
-XENKBD_FIELD_FEAT_MTOUCH, 0);
-   if (touch) {
+   with_ptr = !xenbus_read_unsigned(dev->nodename,
+XENKBD_FIELD_FEAT_DSBL_POINTER, 0);
+
+   /* Direct logic: if set, then create multi-touch device. */
+   with_mtouch = xenbus_read_unsigned(dev->nodename,
+  XENKBD_FIELD_FEAT_MTOUCH, 0);
+   if (with_mtouch) {
ret = xenbus_write(XBT_NIL, dev->nodename,
   XENKBD_FIELD_REQ_MTOUCH, "1");
if (ret) {
pr_warn("xenkbd: can't request multi-touch");
-   touch = 0;
+   with_mtouch = 0;
}
}

Does it make sense to still end up calling xenkbd_connect_backend() when
all interfaces (keyboard, pointer, and multitouch) are disabled? Should
we do:

if (!(with_kbd || || with_ptr || with_mtouch))
return -ENXIO;

?

It does make sense. Then we probably need to move all xenbus_read_unsigned
calls to the very beginning of the .probe, so no memory allocations are made
which will be useless if we return -ENXIO, e.g. something like

static int xenkbd_probe(struct xenbus_device *dev,
                   const struct xenbus_device_id *id)
{
     int ret, i;
     bool with_mtouch, with_kbd, with_ptr;
     struct xenkbd_info *info;
     struct input_dev *kbd, *ptr, *mtouch;



if (!(with_kbd | with_ptr | with_mtouch))
         return -ENXIO;

Does the above looks ok?

Yes. Another option is to keep the check where I suggested and do

if (...) {
ret = -ENXIO;
goto error;
}

Whichever you prefer is fine with me.

I will go with the change you suggested and
I'll send v4 tomorrow then.

Thanks.


Thank you,
Oleksandr

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

Re: [Xen-devel] [PATCH v4 1/2] xen/kbdif: Add string constants for raw pointer

[Oleksandr Andrushchenko]

On 05/17/2018 07:28 AM, Juergen Gross wrote:

On 16/05/18 06:59, Oleksandr Andrushchenko wrote:

On 05/11/2018 06:15 PM, Juergen Gross wrote:

On 11/05/18 15:38, Konrad Rzeszutek Wilk wrote:

On Fri, May 11, 2018 at 09:37:57AM -0400, Konrad Rzeszutek Wilk wrote:

On Wed, May 02, 2018 at 05:49:18PM +0300, Oleksandr Andrushchenko
wrote:

From: Oleksandr Andrushchenko 

Add missing string constants for {feature|request}-raw-pointer
to align with the rest of the interface file.

Fixes 7868654ff7fe ("kbdif: Define "feature-raw-pointer" and
"request-raw-pointer")

Signed-off-by: Oleksandr Andrushchenko


Reviewed-by: Konrad Rzeszutek Wilk 

Juergen, you OK with an release-ack?

Yes:

Release-acked-by: Juergen Gross 

Is this ack for both patches or this one only?

It was meant for patch 1 only.

Clear

I see Konrad has applied this patch, but
"[Xen-devel][PATCH v4 2/2] xen/kbdif: Add features to disable keyboard
and pointer"
is still floating.

In principle I'm fine with taking it for 4.11, but I think at this stage
of the release there should be a formal request to include it.

...and the request should come from Konrad?
Or who else may request that?

And, as I have r-b's from Konrad for both patches,
will it be ok if we apply the corresponding changes to the
kernel now, so my change to xen-kbdfront is unblocked?


Juergen

Thank you,
Oleksandr

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

Re: [Xen-devel] [PATCH v4 1/2] xen/kbdif: Add string constants for raw pointer

[Juergen Gross]

On 16/05/18 06:59, Oleksandr Andrushchenko wrote:
> On 05/11/2018 06:15 PM, Juergen Gross wrote:
>> On 11/05/18 15:38, Konrad Rzeszutek Wilk wrote:
>>> On Fri, May 11, 2018 at 09:37:57AM -0400, Konrad Rzeszutek Wilk wrote:
 On Wed, May 02, 2018 at 05:49:18PM +0300, Oleksandr Andrushchenko
 wrote:
> From: Oleksandr Andrushchenko 
>
> Add missing string constants for {feature|request}-raw-pointer
> to align with the rest of the interface file.
>
> Fixes 7868654ff7fe ("kbdif: Define "feature-raw-pointer" and
> "request-raw-pointer")
>
> Signed-off-by: Oleksandr Andrushchenko
> 

 Reviewed-by: Konrad Rzeszutek Wilk 
>>> Juergen, you OK with an release-ack?
>> Yes:
>>
>> Release-acked-by: Juergen Gross 
> Is this ack for both patches or this one only?

It was meant for patch 1 only.

> I see Konrad has applied this patch, but
> "[Xen-devel][PATCH v4 2/2] xen/kbdif: Add features to disable keyboard
> and pointer"
> is still floating.

In principle I'm fine with taking it for 4.11, but I think at this stage
of the release there should be a formal request to include it.


Juergen

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

Re: [Xen-devel] [xen-unstable test] 122804: FAIL

[Juergen Gross]

On 17/05/18 01:57, osstest service owner wrote:
> flight 122804 xen-unstable real [real]
> http://logs.test-lab.xenproject.org/osstest/logs/122804/
> 
> Failures and problems with tests :-(
> 
> Tests which did not succeed and are blocking,
> including tests which could not be run:
>  test-arm64-arm64-xl  broken  in 
> 122715
>  test-arm64-arm64-xl-xsm  broken  in 
> 122715

Those were on laxton1.

> 
> Tests which are failing intermittently (not blocking):
>  test-amd64-amd64-xl-qemut-debianhvm-amd64 16 guest-localmigrate/x10 fail 
> pass in 122715
>  test-amd64-amd64-xl-qemuu-debianhvm-amd64 16 guest-localmigrate/x10 fail 
> pass in 122715

Not sure about those. Olaf has seen a similar problem in our SLE15 tests
which seems to be related to qemu file locking issues. At least the logs
look exactly the same. See the thread

https://lists.xen.org/archives/html/xen-devel/2018-05/msg00369.html


Juergen

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

[Xen-devel] [xen-unstable test] 122804: FAIL

[osstest service owner]

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

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-arm64-arm64-xl  broken  in 122715
 test-arm64-arm64-xl-xsm  broken  in 122715

Tests which are failing intermittently (not blocking):
 test-arm64-arm64-examine  5 host-install   broken in 122715 pass in 122804
 test-arm64-arm64-xl  4 host-install(4) broken in 122715 pass in 122804
 test-arm64-arm64-xl-xsm  4 host-install(4) broken in 122715 pass in 122804
 test-amd64-amd64-xl-qemut-debianhvm-amd64 16 guest-localmigrate/x10 fail pass 
in 122715
 test-amd64-amd64-xl-qemuu-debianhvm-amd64 16 guest-localmigrate/x10 fail pass 
in 122715
 test-armhf-armhf-xl-arndale  19 leak-check/check   fail pass in 122715

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

[Xen-devel] Test for osstest, features used in Qubes OS

[Marek Marczykowski-Górecki]

Hi,

As discussed some time ago, I'd like to help with adding tests for some
features we use in Qubes OS.

IMO the easiest thing to test is host suspend. You just need to execute
"rtcwake -s 30 -m mem", and see if the host is back to live after ~30s.
Right now I know it works on Xen 4.8, but supposedly is broken on
staging (haven't tested the most recent version).
Next step would be the same while having some domains running.

How the test should look like (where to add this? etc)?

Next things would be mostly related to PCI passthrough:
 - PCI passthrough with qemu in stubdomain
 - the same as above, but with Linux-based stubdomain (we need cleanup
   and send patches for that first, probably 4.12 material)
 - guest suspend (recently added libxl_domain_suspend_only), for
   different guest types (PV, PVH, HVM), also with/without PCI device

For this, the machine obviously need to have IOMMU (I assume at least
some of the hardware used in test lab have it), and some spare PCI
device. I use sound card for some of such tests. But testing on USB
controllers would be more useful (from out experience, one of the most
problematic devices for suspend, sadly also lacking FLR or such...).

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


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

Re: [Xen-devel] [PATCH v3 2/2] Input: xen-kbdfront - allow better run-time configuration

[Dmitry Torokhov]

On Wed, May 16, 2018 at 08:47:30PM +0300, Oleksandr Andrushchenko wrote:
> On 05/16/2018 08:15 PM, Dmitry Torokhov wrote:
> > Hi Oleksandr,
> > 
> > On Mon, May 14, 2018 at 05:40:29PM +0300, Oleksandr Andrushchenko wrote:
> > > @@ -211,93 +220,114 @@ static int xenkbd_probe(struct xenbus_device *dev,
> > >   if (!info->page)
> > >   goto error_nomem;
> > > - /* Set input abs params to match backend screen res */
> > > - abs = xenbus_read_unsigned(dev->otherend,
> > > -XENKBD_FIELD_FEAT_ABS_POINTER, 0);
> > > - ptr_size[KPARAM_X] = xenbus_read_unsigned(dev->otherend,
> > > -   XENKBD_FIELD_WIDTH,
> > > -   ptr_size[KPARAM_X]);
> > > - ptr_size[KPARAM_Y] = xenbus_read_unsigned(dev->otherend,
> > > -   XENKBD_FIELD_HEIGHT,
> > > -   ptr_size[KPARAM_Y]);
> > > - if (abs) {
> > > - ret = xenbus_write(XBT_NIL, dev->nodename,
> > > -XENKBD_FIELD_REQ_ABS_POINTER, "1");
> > > - if (ret) {
> > > - pr_warn("xenkbd: can't request abs-pointer\n");
> > > - abs = 0;
> > > - }
> > > - }
> > > + /*
> > > +  * The below are reverse logic, e.g. if the feature is set, then
> > > +  * do not expose the corresponding virtual device.
> > > +  */
> > > + with_kbd = !xenbus_read_unsigned(dev->nodename,
> > > +  XENKBD_FIELD_FEAT_DSBL_KEYBRD, 0);
> > > - touch = xenbus_read_unsigned(dev->nodename,
> > > -  XENKBD_FIELD_FEAT_MTOUCH, 0);
> > > - if (touch) {
> > > + with_ptr = !xenbus_read_unsigned(dev->nodename,
> > > +  XENKBD_FIELD_FEAT_DSBL_POINTER, 0);
> > > +
> > > + /* Direct logic: if set, then create multi-touch device. */
> > > + with_mtouch = xenbus_read_unsigned(dev->nodename,
> > > +XENKBD_FIELD_FEAT_MTOUCH, 0);
> > > + if (with_mtouch) {
> > >   ret = xenbus_write(XBT_NIL, dev->nodename,
> > >  XENKBD_FIELD_REQ_MTOUCH, "1");
> > >   if (ret) {
> > >   pr_warn("xenkbd: can't request multi-touch");
> > > - touch = 0;
> > > + with_mtouch = 0;
> > >   }
> > >   }
> > Does it make sense to still end up calling xenkbd_connect_backend() when
> > all interfaces (keyboard, pointer, and multitouch) are disabled? Should
> > we do:
> > 
> > if (!(with_kbd || || with_ptr || with_mtouch))
> > return -ENXIO;
> > 
> > ?
> It does make sense. Then we probably need to move all xenbus_read_unsigned
> calls to the very beginning of the .probe, so no memory allocations are made
> which will be useless if we return -ENXIO, e.g. something like
> 
> static int xenkbd_probe(struct xenbus_device *dev,
>                   const struct xenbus_device_id *id)
> {
>     int ret, i;
>     bool with_mtouch, with_kbd, with_ptr;
>     struct xenkbd_info *info;
>     struct input_dev *kbd, *ptr, *mtouch;
> 
> 
> 
> if (!(with_kbd | with_ptr | with_mtouch))
>         return -ENXIO;
> 
> Does the above looks ok?

Yes. Another option is to keep the check where I suggested and do

if (...) {
ret = -ENXIO;
goto error;
}

Whichever you prefer is fine with me.

Thanks.

-- 
Dmitry

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

Re: [Xen-devel] [PATCH v7 0/7] KVM: x86: Allow Qemu/KVM to use PVH entry point

[Maran Wilson]

Friendly ping. I am hopeful one of the x86 and/or KVM maintainers has a 
few cycles to spare to look this over.


And thanks to everyone who has helped thus far by providing valuable 
feedback and reviewing.


   https://lkml.org/lkml/2018/4/16/1002

Thanks,
-Maran

On 4/16/2018 4:09 PM, Maran Wilson wrote:

For certain applications it is desirable to rapidly boot a KVM virtual
machine. In cases where legacy hardware and software support within the
guest is not needed, Qemu should be able to boot directly into the
uncompressed Linux kernel binary without the need to run firmware.

There already exists an ABI to allow this for Xen PVH guests and the ABI
is supported by Linux and FreeBSD:

https://xenbits.xen.org/docs/unstable/misc/pvh.html

This patch series would enable Qemu to use that same entry point for
booting KVM guests.

Changes from v6:

  * Addressed issues caught by the kbuild test robot:
 - Restored an #include line that had been dropped by mistake (patch 4)
 - Removed a pair of #include lines that were no longer needed in a
   common code file and causing problems for certain 32-bit configs
   (patchs 4 and 7)

Changes from v5:

  * The interface changes to the x86/HVM start info layout have
now been accepted into the Xen tree.
  * Rebase and merge upstream PVH file changes.
  * (Patch 6) Synced up to the final version of the header file that was
  acked and pulled into the Xen tree.
  * (Patch 1) Fixed typo and removed redundant "def_bool n" line.

Changes from v4:

Note: I've withheld Juergen's earlier "Reviewed-by" tags from patches
1 and 7 since there were minor changes (mostly just addition of
CONFIG_KVM_GUEST_PVH as requested) that came afterwards.

  * Changed subject prefix from RFC to PATCH
  * Added CONFIG_KVM_GUEST_PVH as suggested
  * Relocated the PVH common files to
arch/x86/platform/pvh/{enlighten.c,head.S}
  * Realized I also needed to move the objtool override for those files
  * Updated a few code comments per reviewer feedback
  * Sent out a patch of the hvm_start_info struct changes against the Xen
tree since that is the canonical copy of the header. Discussions on
that thread have resulted in some (non-functional) updates to
start_info.h (patch 6/7) and those changes are reflected here as well
in order to keep the files in sync. The header file has since been
ack'ed for the Xen tree by Jan Beulich.

Changes from v3:

  * Implemented Juergen's suggestion for refactoring and moving the PVH
code so that CONFIG_XEN is no longer required for booting KVM guests
via the PVH entry point.
Functionally, nothing has changed from V3 really, but the patches
look completely different now because of all the code movement and
refactoring. Some of these patches can be combined, but I've left
them very small in some cases to make the refactoring and code
movement easier to review.
My approach for refactoring has been to create a PVH entry layer that
still has understanding and knowledge about Xen vs non-Xen guest types
so that it can make run time decisions to handle either case, as
opposed to going all the way and re-writing it to be a completely
hypervisor agnostic and architecturally pure layer that is separate
from guest type details. The latter seemed a bit overkill in this
situation. And I've handled the complexity of having to support
Qemu/KVM boot of kernels compiled with or without CONFIG_XEN via a
pair of xen specific __weak routines that can be overridden in kernels
that support Xen guests. Importantly, the __weak routines are for
xen specific code only (not generic "guest type" specific code) so
there is no clashing between xen version of the strong routine and,
say, a KVM version of the same routine. But I'm sure there are many
ways to skin this cat, so I'm open to alternate suggestions if there
is a compelling reason for not using __weak in this situation.

Changes from v2:

  * All structures (including memory map table entries) are padded and
aligned to an 8 byte boundary.

  * Removed the "packed" attributes and made changes to comments as
suggested by Jan.

Changes from v1:

  * Adopted Paolo's suggestion for defining a v2 PVH ABI that includes the
e820 map instead of using the second module entry to pass the table.

  * Cleaned things up a bit to reduce the number of xen vs non-xen special
cases.


Maran Wilson (7):
   xen/pvh: Split CONFIG_XEN_PVH into CONFIG_PVH and CONFIG_XEN_PVH
   xen/pvh: Move PVH entry code out of Xen specific tree
   xen/pvh: Create a new file for Xen specific PVH code
   xen/pvh: Move Xen specific PVH VM initialization out of common file
   xen/pvh: Move Xen code for getting mem map via hcall out of common
 file
   xen/pvh: Add memory map pointer to hvm_start_info struct
   KVM: x86: Allow Qemu/KVM to use PVH entry point

  MAINTAINERS |   1 +
  

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qemuu-win7-amd64 13 guest-saverestore fail REGR. vs. 122696
 test-armhf-armhf-examine  8 reboot   fail REGR. vs. 122696
 test-armhf-armhf-xl-multivcpu  7 xen-bootfail REGR. vs. 122696
 test-armhf-armhf-libvirt  7 xen-boot fail REGR. vs. 122696
 test-armhf-armhf-xl-vhd   7 xen-boot fail REGR. vs. 122696
 test-armhf-armhf-libvirt-raw  7 xen-boot fail REGR. vs. 122696
 test-armhf-armhf-xl-cubietruck  7 xen-boot   fail REGR. vs. 122696
 test-armhf-armhf-xl  10 debian-install   fail REGR. vs. 122696

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

version targeted for testing:
 linuxaf37b24ae6b3e7eac272417c9af591c58baa8f93
baseline version:
 linux94d7dbf108813ea45a91e27e9a8bd231d5a23fa7

Last test of basis  (not found) 
Failing since   (not found) 
Testing same since   122796  2018-05-14 09:18:54 Z2 days1 attempts

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

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

[osstest service owner]

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

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  e1f912cbf7178798b0646c7d0753b8d67e139e75
baseline version:
 xen  a65179dedd6415134029de00a17c218af647fb1a

Last test of basis   122879  2018-05-16 15:01:26 Z0 days
Testing same since   122888  2018-05-16 18:02:08 Z0 days1 attempts


People who touched revisions under test:
  George Dunlap 
  Ian Jackson 
  Lars Kurth 

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



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

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

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

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


Pushing revision :

To xenbits.xen.org:/home/xen/git/xen.git
   a65179dedd..e1f912cbf7  e1f912cbf7178798b0646c7d0753b8d67e139e75 -> smoke

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

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

[osstest service owner]

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

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  a65179dedd6415134029de00a17c218af647fb1a
baseline version:
 xen  3352afc26c497d26ecb70527db3cb29daf7b1422

Last test of basis   122877  2018-05-16 13:00:44 Z0 days
Testing same since   122879  2018-05-16 15:01:26 Z0 days1 attempts


People who touched revisions under test:
  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-i386 pass
 test-amd64-amd64-libvirt pass



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

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

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

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


Pushing revision :

To xenbits.xen.org:/home/xen/git/xen.git
   3352afc26c..a65179dedd  a65179dedd6415134029de00a17c218af647fb1a -> smoke

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

Re: [Xen-devel] [PATCH for-4.7/4.8] x86: Fix "x86: further CPUID handling adjustments"

[Andrew Cooper]

On 16/05/18 09:14, Jan Beulich wrote:
 On 15.05.18 at 19:54,  wrote:
>> Also, I don't see any link between the change and the commit message.  With
>> the microcode installed, STIBP and IBPB are already visible to dom0.
> They reportedly weren't (and I was able to confirm that), and given this
> original (prior to that change) code
>
> }
> }
> else
> b = c = 0;
> a = d = 0;
>
> I also can't see how IBRSB and STIBP could have been visible. I agree I
> had wrongly extended that to IBPB.
>
>> The only required adjustment is to force STIBP == IBRSB, which must be done
>> after applying the pv_featureset[] mask to the toolstack's choice of value.
> I can see how I've got that part wrong from a leveling perspective (I was
> really too focused on Dom0 back then), but I don't see how reporting IBPB
> when IBRSB is available in hardware (implying IBPB itself isn't) would work
> with your change in place.

I've submitted v2 with an updated commit message, now I understand where
the dom0 comment came from.

>
> I'm also not convinced assimilating Sergey's original change into this one is
> appropriate - raw_featureset[] isn't used for anything except the sysctl.

You regressed a feature with an incorrect backport, in a way which
directly impacts a tool which administrators will use to see if they've
got the microcode applied properly.

It was broken in a security patch, therefore it is going to get fixed.

~Andrew

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

Re: [Xen-devel] [PATCH v3 2/2] Input: xen-kbdfront - allow better run-time configuration

[Oleksandr Andrushchenko]

On 05/16/2018 08:15 PM, Dmitry Torokhov wrote:

Hi Oleksandr,

On Mon, May 14, 2018 at 05:40:29PM +0300, Oleksandr Andrushchenko wrote:

@@ -211,93 +220,114 @@ static int xenkbd_probe(struct xenbus_device *dev,
if (!info->page)
goto error_nomem;
  
-	/* Set input abs params to match backend screen res */

-   abs = xenbus_read_unsigned(dev->otherend,
-  XENKBD_FIELD_FEAT_ABS_POINTER, 0);
-   ptr_size[KPARAM_X] = xenbus_read_unsigned(dev->otherend,
- XENKBD_FIELD_WIDTH,
- ptr_size[KPARAM_X]);
-   ptr_size[KPARAM_Y] = xenbus_read_unsigned(dev->otherend,
- XENKBD_FIELD_HEIGHT,
- ptr_size[KPARAM_Y]);
-   if (abs) {
-   ret = xenbus_write(XBT_NIL, dev->nodename,
-  XENKBD_FIELD_REQ_ABS_POINTER, "1");
-   if (ret) {
-   pr_warn("xenkbd: can't request abs-pointer\n");
-   abs = 0;
-   }
-   }
+   /*
+* The below are reverse logic, e.g. if the feature is set, then
+* do not expose the corresponding virtual device.
+*/
+   with_kbd = !xenbus_read_unsigned(dev->nodename,
+XENKBD_FIELD_FEAT_DSBL_KEYBRD, 0);
  
-	touch = xenbus_read_unsigned(dev->nodename,

-XENKBD_FIELD_FEAT_MTOUCH, 0);
-   if (touch) {
+   with_ptr = !xenbus_read_unsigned(dev->nodename,
+XENKBD_FIELD_FEAT_DSBL_POINTER, 0);
+
+   /* Direct logic: if set, then create multi-touch device. */
+   with_mtouch = xenbus_read_unsigned(dev->nodename,
+  XENKBD_FIELD_FEAT_MTOUCH, 0);
+   if (with_mtouch) {
ret = xenbus_write(XBT_NIL, dev->nodename,
   XENKBD_FIELD_REQ_MTOUCH, "1");
if (ret) {
pr_warn("xenkbd: can't request multi-touch");
-   touch = 0;
+   with_mtouch = 0;
}
}

Does it make sense to still end up calling xenkbd_connect_backend() when
all interfaces (keyboard, pointer, and multitouch) are disabled? Should
we do:

if (!(with_kbd || || with_ptr || with_mtouch))
return -ENXIO;

?

It does make sense. Then we probably need to move all xenbus_read_unsigned
calls to the very beginning of the .probe, so no memory allocations are made
which will be useless if we return -ENXIO, e.g. something like

static int xenkbd_probe(struct xenbus_device *dev,
                  const struct xenbus_device_id *id)
{
    int ret, i;
    bool with_mtouch, with_kbd, with_ptr;
    struct xenkbd_info *info;
    struct input_dev *kbd, *ptr, *mtouch;



if (!(with_kbd | with_ptr | with_mtouch))
        return -ENXIO;

Does the above looks ok?

Thanks.


Thank you,
Oleksandr

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

[Xen-devel] [libvirt test] 122801: tolerable all pass - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 122561
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 122561
 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail  like 122561
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-arm64-arm64-libvirt 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-qcow2 12 migrate-support-checkfail never pass
 test-arm64-arm64-libvirt-qcow2 13 saverestore-support-checkfail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail   never pass

version targeted for testing:
 libvirt  2c4affd57e1746183ff410c023face80866bbe0f
baseline version:
 libvirt  764a7483f189e6de841163647c14296e693dbb2e

Last test of basis   122561  2018-05-02 10:08:33 Z   14 days
Failing since122567  2018-05-03 04:18:54 Z   13 days8 attempts
Testing same since   122801  2018-05-14 10:11:33 Z2 days1 attempts


People who touched revisions under test:
  Andrea Bolognani 
  Clementine Hayat 
  Cole Robinson 
  Daniel P. Berrangé 
  Daniel Veillard 
  David Kiarie 
  Eric Blake 
  Erik Skultety 
  Fabian Freyer 
  Igor Gnatenko 
  John Ferlan 
  Julio Faracco 
  Ján Tomko 
  Lin Ma 
  Maciej Wolny 
  Martin Kletzander 
  Michal Privoznik 
  Peter Krempa 
  Prafullkumar Tale 
  Roland Schulz 
  Stefan Berger 
  Ville Skyttä 

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-arm64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-arm64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-arm64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass
 test-amd64-amd64-libvirt-xsm pass
 test-arm64-arm64-libvirt-xsm pass
 test-armhf-armhf-libvirt-xsm pass
 test-amd64-i386-libvirt-xsm  pass
 test-amd64-amd64-libvirt pass
 test-arm64-arm64-libvirt pass
 test-armhf-armhf-libvirt pass
 test-amd64-i386-libvirt  pass
 test-amd64-amd64-libvirt-pair 

[Xen-devel] [PATCH v2 for-4.7/4.8] x86: Fix "x86: further CPUID handling adjustments"

[Andrew Cooper]

c/s f9616884e (a backport of c/s 0d703a701 "x86/feature: Definitions for
Indirect Branch Controls") missed a CPUID adjustment when calculating the raw
featureset.  This impacts host administrator diagnostics.

Signed-off-by: Sergey Dyasli 

c/s 62b187969 "x86: further CPUID handling adjustments" make some adjustments.
However, it breaks levelling of guests, making it impossible for the toolstack
to hide STIBP or IBPB from guests on hardware with up-to-date microcode.

The dom0 issue referenced in the commit message was fixed by the hunk
adjusting the zeroing alone.  STIBP and IBPB don't need (and indeed, must not
be for levelling purposes) OR'd into the leaf.

One final item which was missed in backport was the need to ignore the
toolstack choice of STIBP, and set it equal to IBRSB.  This needs doing after
the mask has been applied.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 

v2:
 * Update the commit message, how I understand the dom0 aspect of the previous
   commit message.
---
 xen/arch/x86/cpuid.c   | 2 +-
 xen/arch/x86/hvm/hvm.c | 8 +---
 xen/arch/x86/traps.c   | 8 +---
 3 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index 451952c..fffcecd 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -113,7 +113,7 @@ static void __init calculate_raw_featureset(void)
 cpuid_count(0x7, 0, ,
 _featureset[FEATURESET_7b0],
 _featureset[FEATURESET_7c0],
-);
+_featureset[FEATURESET_7d0]);
 if ( max >= 0xd )
 cpuid_count(0xd, 1,
 _featureset[FEATURESET_Da1],
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index ff1c6fa..0a1d4a9 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -3496,10 +3496,13 @@ void hvm_cpuid(unsigned int input, unsigned int *eax, 
unsigned int *ebx,
  special_features[FEATURESET_7b0]);
 
 *ecx &= hvm_featureset[FEATURESET_7c0];
-
-*edx |= cpufeat_mask(X86_FEATURE_STIBP);
 *edx &= hvm_featureset[FEATURESET_7d0];
 
+/* Force STIBP equal to IBRSB */
+*edx &= ~cpufeat_mask(X86_FEATURE_STIBP);
+if ( *edx & cpufeat_mask(X86_FEATURE_IBRSB) )
+*edx |= cpufeat_mask(X86_FEATURE_STIBP);
+
 /* Don't expose HAP-only features to non-hap guests. */
 if ( !hap_enabled(d) )
 {
@@ -3657,7 +3660,6 @@ void hvm_cpuid(unsigned int input, unsigned int *eax, 
unsigned int *ebx,
 hvm_cpuid(0x8001, NULL, NULL, NULL, &_edx);
 *eax |= (_edx & cpufeat_mask(X86_FEATURE_LM) ? vaddr_bits : 32) << 8;
 
-*ebx |= cpufeat_mask(X86_FEATURE_IBPB);
 *ebx &= hvm_featureset[FEATURESET_e8b];
 break;
 }
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 0f34b21..da26749 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1088,10 +1088,13 @@ void pv_cpuid(struct cpu_user_regs *regs)
   special_features[FEATURESET_7b0]);
 
 c &= pv_featureset[FEATURESET_7c0];
-
-d |= cpufeat_mask(X86_FEATURE_STIBP);
 d &= pv_featureset[FEATURESET_7d0];
 
+/* Force STIBP equal to IBRSB */
+d &= ~cpufeat_mask(X86_FEATURE_STIBP);
+if ( d & cpufeat_mask(X86_FEATURE_IBRSB) )
+d |= cpufeat_mask(X86_FEATURE_STIBP);
+
 if ( !is_pvh_domain(currd) )
 {
 /*
@@ -1188,7 +1191,6 @@ void pv_cpuid(struct cpu_user_regs *regs)
 
 case 0x8008:
 a = paddr_bits | (vaddr_bits << 8);
-b |= cpufeat_mask(X86_FEATURE_IBPB);
 b &= pv_featureset[FEATURESET_e8b];
 break;
 
-- 
2.1.4


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

Re: [Xen-devel] [PATCH v3 2/2] Input: xen-kbdfront - allow better run-time configuration

[Dmitry Torokhov]

Hi Oleksandr,

On Mon, May 14, 2018 at 05:40:29PM +0300, Oleksandr Andrushchenko wrote:
> @@ -211,93 +220,114 @@ static int xenkbd_probe(struct xenbus_device *dev,
>   if (!info->page)
>   goto error_nomem;
>  
> - /* Set input abs params to match backend screen res */
> - abs = xenbus_read_unsigned(dev->otherend,
> -XENKBD_FIELD_FEAT_ABS_POINTER, 0);
> - ptr_size[KPARAM_X] = xenbus_read_unsigned(dev->otherend,
> -   XENKBD_FIELD_WIDTH,
> -   ptr_size[KPARAM_X]);
> - ptr_size[KPARAM_Y] = xenbus_read_unsigned(dev->otherend,
> -   XENKBD_FIELD_HEIGHT,
> -   ptr_size[KPARAM_Y]);
> - if (abs) {
> - ret = xenbus_write(XBT_NIL, dev->nodename,
> -XENKBD_FIELD_REQ_ABS_POINTER, "1");
> - if (ret) {
> - pr_warn("xenkbd: can't request abs-pointer\n");
> - abs = 0;
> - }
> - }
> + /*
> +  * The below are reverse logic, e.g. if the feature is set, then
> +  * do not expose the corresponding virtual device.
> +  */
> + with_kbd = !xenbus_read_unsigned(dev->nodename,
> +  XENKBD_FIELD_FEAT_DSBL_KEYBRD, 0);
>  
> - touch = xenbus_read_unsigned(dev->nodename,
> -  XENKBD_FIELD_FEAT_MTOUCH, 0);
> - if (touch) {
> + with_ptr = !xenbus_read_unsigned(dev->nodename,
> +  XENKBD_FIELD_FEAT_DSBL_POINTER, 0);
> +
> + /* Direct logic: if set, then create multi-touch device. */
> + with_mtouch = xenbus_read_unsigned(dev->nodename,
> +XENKBD_FIELD_FEAT_MTOUCH, 0);
> + if (with_mtouch) {
>   ret = xenbus_write(XBT_NIL, dev->nodename,
>  XENKBD_FIELD_REQ_MTOUCH, "1");
>   if (ret) {
>   pr_warn("xenkbd: can't request multi-touch");
> - touch = 0;
> + with_mtouch = 0;
>   }
>   }

Does it make sense to still end up calling xenkbd_connect_backend() when
all interfaces (keyboard, pointer, and multitouch) are disabled? Should
we do:

if (!(with_kbd || || with_ptr || with_mtouch))
return -ENXIO;

?

Thanks.

-- 
Dmitry

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

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-i386-libvirt6 libvirt-buildfail REGR. vs. 122667

version targeted for testing:
 ovmf 989f7a2cf0e27123fda5ca538b15832e115e0f4e
baseline version:
 ovmf 0edb7ec5ced0a28b93bf8c13b12f0a277c44dbbc

Last test of basis   122667  2018-05-09 06:53:09 Z7 days
Failing since122718  2018-05-12 06:34:08 Z4 days2 attempts
Testing same since   122797  2018-05-14 09:29:39 Z2 days1 attempts


People who touched revisions under test:
  Ansen Huang 
  Bi, Dandan 
  Carsey, Jaben 
  cinnamon shia 
  Dandan Bi 
  Derek Lin 
  Jaben Carsey 
  Lin, Derek 
  Star Zeng 
  Yonghong Zhu 
  Yunhua Feng 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   fail
 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


Not pushing.


commit 989f7a2cf0e27123fda5ca538b15832e115e0f4e
Author: cinnamon shia 
Date:   Fri May 11 23:21:12 2018 +0800

MdeModulePkg Variable: Fix the returned status in UpdateVariableStore

If Fvb is a NULL, return EFI_UNSUPPORTED.
If the remaining size is not enough, return EFI_OUT_OF_RESOURCES.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: cinnamon shia 
Signed-off-by: Ansen Huang 
Reviewed-by: Star Zeng 

commit d741d1419350dc4ae868d4a9396f9da33a3416a0
Author: cinnamon shia 
Date:   Fri May 11 23:21:11 2018 +0800

MdeModulePkg Variable: Fix a corner case issue about setting a variable

Fix the issue that failed to update or add a UEFI variable if the remaining 
size is equal to the data size
of the variable.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: cinnamon shia 
Signed-off-by: Ansen Huang 
Reviewed-by: Star Zeng 

commit c61db18e5d11e4c25e32bfb3f999a88e3207eb5f
Author: Lin, Derek 
Date:   Wed May 9 17:03:24 2018 +0800

BaseTools: Fix python error with --genfds-multi-thread.

When self.Alignment is None, it ran into python error since there is no
strip() in None.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Derek Lin 
Reviewed-by: Yonghong Zhu 

commit c731b5450576a8dd0afb1d2dd013203235395a4f
Author: Yonghong Zhu 
Date:   Wed May 9 16:41:28 2018 +0800

BaseTools: Remove the redundant code

the ArraySize and Array already be got in line 1093, so this code are
redundant.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu 
Reviewed-by: Jaben Carsey 

commit 6b285ca3663d3c71cc68d01684a98f0d7537885e
Author: Yunhua Feng 
Date:   Mon May 7 18:26:24 2018 +0800

BaseTools: Fix generating array's size is incorrect in AutoGen.c

case example:
DSC:
 [PcdsFixedAtBuild]
  PcdToken.PcdName | "A"
 [Components]
 TestPkg/TestDriver.inf {
  PcdToken.PcdName | {0x41,0x42,0x43,0x44}
 }

Generating the size of array is incorrect in AutoGen.c

Re: [Xen-devel] backporting considerations (Re: [PATCH v9 0/9] xen/x86: various XPTI speedups)

[George Dunlap]

On Wed, May 16, 2018 at 5:01 PM, Jan Beulich  wrote:
 On 16.05.18 at 16:53,  wrote:
>> On Wed, May 16, 2018 at 3:01 PM, Jan Beulich  wrote:
>> On 16.05.18 at 15:18,  wrote:
 If the latter, I think the same argument applies: turning on XPTI is a
 requirement for many people, and thus represents a pretty hefty
 performance regression.  While we don't need to backport normal fixes
 to security-only releases, we should certainly try to avoid
 regressions.
>>>
>>> I don't think we would have addressed non-security fallout (or other
>>> than really severe regressions) from other security patches in the
>>> past on security only branches. People caring about performance
>>> should upgrade.
>>
>> If a security patch, when backported to 4.6, broke some fairly
>> critical bit of functionality (say,  openvswitch support), you would
>> oppose a subsequent patch which would fix that regression?
>>
>> That doesn't seem very reasonable to me.  Users shouldn't have to
>> choose between being vulnerable to a security issue and losing
>> functionality which was working at the last release.  Otherwise,
>> what's the point of having "security supported" releases?
>
> Note how I did say "or other than really severe regressions". I think
> your "fairly critical bit of functionality" falls into exactly that area.

Right, so we agree on the basic principles, but disagree about whether
XPTI's performance hit counts as a "really severe regression".  At
least one of my CentOS users was seriously considering applying
juergen's XPTI improvement patch before it even hit staging, because,
"We're struggling with slowdowns from xpti within the dom0."  (I
advised her not to at that point, because XenRT had flagged up some
potential issues.)  If a user describes herself as "struggling", I
think that counts as a regression.  This was for Xen 4.8, but I think
the basic principle applies.  I'll definitely be backporting those to
the 4.8 CentOS packages now that they're in staging; and I'll probably
try backporting them to the 4.6 packages too (since they're currently
the default, although hopefully not for long).

Like I said, I don't expect you personally to do something you don't
think is worth your time.  But I think it would in general be good if
the XenProject fixed this performance regression for releases which
are still under security support.

 -George

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

Re: [Xen-devel] [PATCH v6] scripts/add_maintainers.pl: New script

[Ian Jackson]

George Dunlap writes ("Re: [PATCH v6] scripts/add_maintainers.pl: New script"):
> I've looked over the help and approve of the functionality, and tested
> it for my use case and it seems to work as advertised.
> 
> On that basis:
> 
> Acked-by: George Dunlap 

Thanks, pushed.

Ian.

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

Re: [Xen-devel] backporting considerations (Re: [PATCH v9 0/9] xen/x86: various XPTI speedups)

[Jan Beulich]

>>> On 16.05.18 at 16:53,  wrote:
> On Wed, May 16, 2018 at 3:01 PM, Jan Beulich  wrote:
> On 16.05.18 at 15:18,  wrote:
>>> If the latter, I think the same argument applies: turning on XPTI is a
>>> requirement for many people, and thus represents a pretty hefty
>>> performance regression.  While we don't need to backport normal fixes
>>> to security-only releases, we should certainly try to avoid
>>> regressions.
>>
>> I don't think we would have addressed non-security fallout (or other
>> than really severe regressions) from other security patches in the
>> past on security only branches. People caring about performance
>> should upgrade.
> 
> If a security patch, when backported to 4.6, broke some fairly
> critical bit of functionality (say,  openvswitch support), you would
> oppose a subsequent patch which would fix that regression?
> 
> That doesn't seem very reasonable to me.  Users shouldn't have to
> choose between being vulnerable to a security issue and losing
> functionality which was working at the last release.  Otherwise,
> what's the point of having "security supported" releases?

Note how I did say "or other than really severe regressions". I think
your "fairly critical bit of functionality" falls into exactly that area.

Jan



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

Re: [Xen-devel] [PATCH v2 05/10] x86/SVM: Add AVIC vmexit handlers

[Jan Beulich]

>>> On 07.05.18 at 23:07,  wrote:
> @@ -180,6 +185,293 @@ int svm_avic_init_vmcb(struct vcpu *v)
>  }
>  
>  /*
> + * Note:
> + * This function handles the AVIC_INCOMP_IPI #vmexit when AVIC is enabled.
> + * The hardware generates this fault when an IPI could not be delivered
> + * to all targeted guest virtual processors because at least one guest
> + * virtual processor was not allocated to a physical core at the time.
> + */
> +void svm_avic_vmexit_do_incomp_ipi(struct cpu_user_regs *regs)
> +{
> +struct vcpu *curr = current;
> +struct domain *currd = curr->domain;
> +struct vmcb_struct *vmcb = curr->arch.hvm_svm.vmcb;
> +u32 icrh = vmcb->exitinfo1 >> 32;
> +u32 icrl = vmcb->exitinfo1;
> +u32 id = vmcb->exitinfo2 >> 32;
> +u32 index = vmcb->exitinfo2 && 0xFF;
> +
> +switch ( id )
> +{
> +case AVIC_INCMP_IPI_ERR_INVALID_INT_TYPE:
> +/*
> + * AVIC hardware handles the delivery of
> + * IPIs when the specified Message Type is Fixed
> + * (also known as fixed delivery mode) and
> + * the Trigger Mode is edge-triggered. The hardware
> + * also supports self and broadcast delivery modes
> + * specified via the Destination Shorthand(DSH)
> + * field of the ICRL. Logical and physical APIC ID
> + * formats are supported. All other IPI types cause
> + * a #VMEXIT, which needs to emulated.

Please utilize the permitted line length (also elsewhere).

> + */
> +vlapic_reg_write(curr, APIC_ICR2, icrh);
> +vlapic_reg_write(curr, APIC_ICR, icrl);
> +break;
> +
> +case AVIC_INCMP_IPI_ERR_TARGET_NOT_RUN:
> +{
> +/*
> + * At this point, we expect that the AVIC HW has already
> + * set the appropriate IRR bits on the valid target
> + * vcpus. So, we just need to kick the appropriate vcpu.
> + */
> +struct vcpu *v;
> +uint32_t dest = GET_xAPIC_DEST_FIELD(icrh);
> +uint32_t short_hand = icrl & APIC_SHORT_MASK;
> +bool dest_mode = icrl & APIC_DEST_MASK;
> +
> +for_each_vcpu ( currd,  v )
> +{
> +if ( v != curr &&
> + vlapic_match_dest(vcpu_vlapic(v), vcpu_vlapic(curr),
> +   short_hand, dest, dest_mode) )
> +{
> +vcpu_kick(v);
> +break;

Why do you break out of the loop here? With a shorthand more than
one vCPU might be the target.

> +}
> +}
> +break;
> +}
> +
> +case AVIC_INCMP_IPI_ERR_INV_TARGET:
> +gprintk(XENLOG_ERR,
> +"SVM: %s: Invalid IPI target (icr=%#08x:%08x, idx=%u)\n",

%#08x produces something like 0x012345, rather than a full eight digits.
Preferably drop the #, or if you really think it's needed replace it be an
explicit 0x.

> +__func__, icrh, icrl, index);

Please use __func__ only when a log message really can't be disambiguated
another way.

For both of these - same further down.

> +static avic_logical_id_entry_t *
> +avic_get_logical_id_entry(struct svm_domain *d, u32 ldr, bool flat)
> +{
> +unsigned int index;
> +unsigned int dest_id = GET_xAPIC_LOGICAL_ID(ldr);
> +
> +if ( !dest_id )
> +return NULL;
> +
> +if ( flat )
> +{
> +index = ffs(dest_id) - 1;
> +if ( index > 7 )
> +return NULL;
> +}
> +else
> +{
> +unsigned int cluster = (dest_id & 0xf0) >> 4;
> +int apic = ffs(dest_id & 0x0f) - 1;
> +
> +if ( (apic < 0) || (apic > 7) || (cluster >= 0xf) )

I can't see a way for apic to be larger than 3 with the calculation above.

> +return NULL;
> +index = (cluster << 2) + apic;
> +}
> +
> +ASSERT(index <= 255);

Which of the many possible meanings of 255 is this?

> +static int avic_ldr_write(struct vcpu *v, u8 g_phy_id, u32 ldr, bool valid)
> +{
> +avic_logical_id_entry_t *entry, new_entry;
> +u32 dfr = vlapic_reg_read(vcpu_vlapic(v), APIC_DFR);

Just to give another example - looks like this too could be vlapic_get_reg().

> +static int avic_handle_ldr_update(struct vcpu *v)
> +{
> +int ret = 0;

Pointless initializer.

> +u32 ldr = vlapic_reg_read(vcpu_vlapic(v), APIC_LDR);
> +u32 apic_id = vlapic_reg_read(vcpu_vlapic(v), APIC_ID);
> +
> +if ( !ldr )
> +return -EINVAL;
> +
> +ret = avic_ldr_write(v, GET_xAPIC_ID(apic_id), ldr, true);
> +if ( ret && v->arch.hvm_svm.avic_last_ldr )
> +{
> +/*
> + * Note:
> + * In case of failure to update LDR register,
> + * we set the guest physical APIC ID to 0,
> + * and set the entry logical APID ID entry
> + * to invalid (false).
> + */
> +avic_ldr_write(v, 0, v->arch.hvm_svm.avic_last_ldr, false);
> +v->arch.hvm_svm.avic_last_ldr = 0;
> +}
> +else
> +{
> +/*
> +

Re: [Xen-devel] [PATCH v2 04/10] x86/HVM/SVM: Add AVIC initialization code

[Andrew Cooper]

On 16/05/18 16:29, Jan Beulich wrote:
 On 07.05.18 at 23:07,  wrote:

>> +s->avic_last_phy_id = avic_get_physical_id_entry(d, 
>> GET_xAPIC_ID(apic_id));
> You don't appear to read this value outside of this function. Please store
> values in struct domain / struct vcpu only if you in fact read them, and
> if their calculation isn't trivial.
>
> I also don't appear to understand the purpose of the "last" in the name.

s->avic_last_phy_id is not needed.  It is a cached unchanging pointer
into the physid table.

Removing it will help clean up some of the later patches.

Strictly speaking, I'm not sure this is true if we decide to permit a
guest to update its local APIC ID, but we don't currently allow this and
I see no benefit from supporting it.

~Andrew

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

Re: [Xen-devel] [PATCH v2 04/10] x86/HVM/SVM: Add AVIC initialization code

[Jan Beulich]

>>> On 07.05.18 at 23:07,  wrote:
> --- /dev/null
> +++ b/xen/arch/x86/hvm/svm/avic.c
> @@ -0,0 +1,190 @@
> +/*
> + * avic.c: implements AMD Advanced Virtual Interrupt Controller (AVIC) 
> support
> + * Copyright (c) 2018, Advanced Micro Devices, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along 
> with
> + * this program; If not, see .
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

Are all of these really needed? For example, xen/stdbool.h isn't commonly
included by non-header files, but is instead obtained from xen/types.h. That
header, in turn, is rarely required to be included explicitly by non-headers
because almost every header already includes it anyway.

In some cases I'm also not convinced you really mean asm/ (rather than
xen/).

> +/* Note: Current max index allowed for physical APIC ID table is 255. */
> +#define AVIC_PHY_APIC_ID_MAXGET_xAPIC_ID(APIC_ID_MASK)

I think it was pointed out before that "max" generally means the last valid
value, rather than the first invalid one.

> +/*
> + * Note:
> + * Currently, svm-avic mode is not supported with nested virtualization.
> + * Therefore, it is not yet currently enabled by default. Once the support
> + * is in-place, this should be enabled by default.
> + */
> +bool svm_avic = false;
> +
> +static const char __section(".bss.page_aligned.const") __aligned(PAGE_SIZE)
> +avic_backing_page[PAGE_SIZE];

So nothing ever writes to this page? I think it would be misleading if CPU side
writes were possible, yet this was marked const.

Also - does this really need allocating statically (rather than just on systems
actually needing it)?

> +static struct avic_physical_id_entry*
> +avic_get_physical_id_entry(struct svm_domain *d, unsigned int index)

I think the first parameter could be const.

> +int svm_avic_dom_init(struct domain *d)
> +{
> +int ret = 0;
> +struct page_info *pg;
> +
> +if ( !svm_avic || !has_vlapic(d) )
> +return 0;
> +
> +/*
> + * Note:
> + * AVIC hardware walks the nested page table to check permissions,
> + * but does not use the SPA address specified in the leaf page
> + * table entry since it uses  address in the AVIC_BACKING_PAGE pointer
> + * field of the VMCB. Therefore, we set up a dummy page for APIC.
> + */
> +set_mmio_p2m_entry(d, paddr_to_pfn(APIC_DEFAULT_PHYS_BASE),
> +   _mfn(virt_to_mfn(avic_backing_page)), PAGE_ORDER_4K,
> +   p2m_access_rw);
> +
> +/* Init AVIC logical APIC ID table */
> +pg = alloc_domheap_page(d, MEMF_no_owner);

Do you really mean d here (and below) rather than NULL?

> +if ( !pg )
> +{
> +ret = -ENOMEM;
> +goto err_out;
> +}
> +clear_domain_page(page_to_mfn(pg));
> +d->arch.hvm_domain.svm.avic_logical_id_table_pg = pg;
> +d->arch.hvm_domain.svm.avic_logical_id_table = 
> __map_domain_page_global(pg);

I think I have said before that I don't think you need to store both
virtual and physical address here, unless both are used frequently.
You establishing a global mapping suggests to me that it's the
virtual address you want to store (MFN and hence struct page_info
can be derived from the mapping via domain_page_map_to_mfn(),
like you already do further down).

> +bool svm_avic_vcpu_enabled(const struct vcpu *v)
> +{
> +const struct arch_svm_struct *s = >arch.hvm_svm;
> +const struct vmcb_struct *vmcb = s->vmcb;
> +
> +return vmcb->_vintr.fields.avic_enable;

Please don't use excess local variables (both of them are used just once,
and I'm sure you could get away with just one of the two [or none at all]
without breaking the line length limit).

Also shouldn't this be vmcb_get_vintr()?

> +int svm_avic_init_vmcb(struct vcpu *v)
> +{
> +u32 apic_id;
> +struct arch_svm_struct *s = >arch.hvm_svm;
> +struct vmcb_struct *vmcb = s->vmcb;
> +struct svm_domain *d = >domain->arch.hvm_domain.svm;
> +const struct vlapic *vlapic = vcpu_vlapic(v);
> +struct avic_physical_id_entry *entry;
> +
> +if ( !svm_avic || !has_vlapic(v->domain) )
> +return 0;
> +
> +if ( !vlapic || !vlapic->regs_page )
> +return -EINVAL;
> +
> +apic_id = vlapic_reg_read(vcpu_vlapic(v), APIC_ID);

Why can't this be 

Re: [Xen-devel] [PATCH v6] scripts/add_maintainers.pl: New script

[George Dunlap]

On 05/16/2018 12:50 PM, Ian Jackson wrote:
> George Dunlap writes ("Re: [PATCH v6] scripts/add_maintainers.pl: New 
> script"):
>>> Changes since v5:
>>> - Add mention of --get-maintainers, and its best use case, to --help
>>>   output.  (Move $get_maintainer up so that it can be used here.)
>>
>> Do you actually want to check this massive changelog into the git repo?
> 
> No.  Lars used a nonstandard form for the `changes in vN' information
> and I decided not to reformat it.  I will strip it out when I commit
> to staging.

Great, thanks. :-)

I've looked over the help and approve of the functionality, and tested
it for my use case and it seems to work as advertised.

On that basis:

Acked-by: George Dunlap 

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

Re: [Xen-devel] [PATCH v2 09/11] docs: Fix some broken references

[Mathieu Poirier]

On 9 May 2018 at 07:18, Mauro Carvalho Chehab
 wrote:
> As we move stuff around, some doc references are broken. Fix some of
> them via this script:
> ./scripts/documentation-file-ref-check --fix-rst
>
> Manually checked if the produced result is valid, removing a few
> false-positives.
>
> Signed-off-by: Mauro Carvalho Chehab 
> ---

>
> diff --git a/Documentation/trace/coresight.txt 
> b/Documentation/trace/coresight.txt
> index 1d74ad0202b6..efbc832146e7 100644
> --- a/Documentation/trace/coresight.txt
> +++ b/Documentation/trace/coresight.txt
> @@ -426,5 +426,5 @@ root@genericarmv8:~#
>  Details on how to use the generic STM API can be found here [2].
>
>  [1]. Documentation/ABI/testing/sysfs-bus-coresight-devices-stm
> -[2]. Documentation/trace/stm.txt
> +[2]. Documentation/trace/stm.rst
>  [3]. https://github.com/Linaro/perf-opencsd

Acked-by: Mathieu Poirier 

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

Re: [Xen-devel] migration regression in xen-4.11 and qemu-2.11 and qcow2

[Olaf Hering]

Am Thu, 10 May 2018 11:40:18 +0100
schrieb Anthony PERARD :

> I did fix the bug in QEMU 2.11 (5d6c599fe1d69a1bf8c5c4d3c58be2b31cd625ad)
> so Xen 4.11 does include it it the qemu-xen tree.

Is this supposed to be called also for PV? In my testing 
qmp_xen_save_devices_state shows up only on HVM.

Olaf


pgpy4rVr6m5oY.pgp
Description: Digitale Signatur von OpenPGP
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

[osstest service owner]

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

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  3352afc26c497d26ecb70527db3cb29daf7b1422
baseline version:
 xen  2adc90908fbb1e614c477e29f2d45eda94570795

Last test of basis   122868  2018-05-16 10:00:53 Z0 days
Testing same since   122877  2018-05-16 13:00:44 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 

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



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

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

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

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


Pushing revision :

To xenbits.xen.org:/home/xen/git/xen.git
   2adc90908f..3352afc26c  3352afc26c497d26ecb70527db3cb29daf7b1422 -> smoke

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

Re: [Xen-devel] backporting considerations (Re: [PATCH v9 0/9] xen/x86: various XPTI speedups)

[George Dunlap]

On Wed, May 16, 2018 at 3:01 PM, Jan Beulich  wrote:
 On 16.05.18 at 15:18,  wrote:
>> On Wed, May 16, 2018 at 10:06 AM, Jan Beulich  wrote:
>> On 26.04.18 at 13:33,  wrote:
 Juergen Gross (9):
   x86/xpti: avoid copying L4 page table contents when possible
   xen/x86: add a function for modifying cr3
   xen/x86: support per-domain flag for xpti
   xen/x86: use invpcid for flushing the TLB
   xen/x86: disable global pages for domains with XPTI active
   xen/x86: use flag byte for decision whether xen_cr3 is valid
   xen/x86: convert pv_guest_cr4_to_real_cr4() to a function
   xen/x86: add some cr3 helpers
   xen/x86: use PCID feature
>>>
>>> This being a performance improvement rather than a plain bug fix series,
>>> I'm not entirely certain about backporting here. My current thinking is to
>>> put this into 4.10 (Jürgen was kind enough to do the backporting work
>>> already), but not into any older trees. Otoh at SUSE we already have
>>> this in our 4.9-based branch as well, and if other consumers of that or
>>> the 4.8 branch would mostly agree it should go there, I could certainly
>>> be convinced.
>>
>> Turning on XPTI causes a pretty large regression in performance; and
>> these reduce that regression significantly.  I'm pretty sure that most
>> downstreams will end up backporting these anyway (I'm sure CentOS
>> will); it's much better to do it officially once, rather than have
>> individual downstreams (most of whom do not have hypervisor developers
>> maintaining packages) all do it separately.
>
> Thanks, recorded as a data point. I don't, however, consider "do not
> have hypervisor developers" as a valid excuse. Package maintainers
> ought to be understanding their packages well enough to be capable of
> doing such backports if they really think they need them. Basically this
> goes back to there being (too many?) downstreams who don't care at
> all to contribute anything back.

I'm not sure exactly who you have in mind here; maybe you mean
downstreams like SuSE and XenServer that sell a product with Xen
inside.  Such companies certainly should have engineers whose job it
is to know the code to one of their key components.

I had in mind downstreams like Fedora, Debian, CentOS, ArchLinux,
XenMadeEasy, and so on; and/or end users who recompile their own
packages based on those; even small projects like QubesOS.  Most of
those people know C and can make a good stab at fixing a patch which
doesn't apply.  But it's not reasonable to expect volunteers
maintaining packages for free, or end users, to be experts in the
hypervisor -- each such port introduces a risk that there will be a
mistake.  And anyway, it seems like a waste of time to make each
downstream using (say) 4.8 duplicate their effort, instead of doing it
once and letting them all share the results.

Having xen packages available for end-users to use *does* contribute
back to the Xen project; and many of the downstreams do contribute
back in many ways, including actual code -- just typically not code in
the hypervisor.

>> If the latter, I think the same argument applies: turning on XPTI is a
>> requirement for many people, and thus represents a pretty hefty
>> performance regression.  While we don't need to backport normal fixes
>> to security-only releases, we should certainly try to avoid
>> regressions.
>
> I don't think we would have addressed non-security fallout (or other
> than really severe regressions) from other security patches in the
> past on security only branches. People caring about performance
> should upgrade.

If a security patch, when backported to 4.6, broke some fairly
critical bit of functionality (say,  openvswitch support), you would
oppose a subsequent patch which would fix that regression?

That doesn't seem very reasonable to me.  Users shouldn't have to
choose between being vulnerable to a security issue and losing
functionality which was working at the last release.  Otherwise,
what's the point of having "security supported" releases?

 -George

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

Re: [Xen-devel] [PATCH v2 03/10] x86/HVM: Make vlapic_reg_read/write() non-static

[Jan Beulich]

>>> On 07.05.18 at 23:07,  wrote:
> --- a/xen/include/asm-x86/hvm/vlapic.h
> +++ b/xen/include/asm-x86/hvm/vlapic.h
> @@ -137,6 +137,10 @@ void vlapic_ipi(struct vlapic *vlapic, uint32_t icr_low, 
> uint32_t icr_high);
>  
>  int vlapic_apicv_write(struct vcpu *v, unsigned int offset);
>  
> +void vlapic_reg_write(struct vcpu *v, unsigned int offset, uint32_t val);
> +
> +uint32_t vlapic_reg_read(const struct vlapic *vlapic, unsigned int offset);

For them to be made non-static, they should really be counterparts of one
another. This (to me) includes suitably matching parameter types (i.e. both
should take struct vlapic * or struct vcpu *; the read one having it const is
of course fine).

Jan



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

Re: [Xen-devel] [PATCH v2 03/10] x86/HVM: Make vlapic_reg_read/write() non-static

[Jan Beulich]

>>> On 07.05.18 at 23:07,  wrote:
> AMD AVIC code makes use of vlapic_reg_read() and vlapic_reg_write(). To
> do this make the functions non-static.

To be honest I'd prefer if each of the two functions was made non-static in
the patch actually needing this to be the case. This allows better judgment
on whether that's really an appropriate thing to do.

Jan



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

Re: [Xen-devel] [PATCH v2 02/10] x86/HVM: Rename vlapic_read_aligned() to vlapic_reg_read()

[Jan Beulich]

>>> On 07.05.18 at 23:07,  wrote:
> Rename vlapic_read_aligned() to vlapic_reg_read() to make it a pair of
> vlapic_reg_write().
> 
> Signed-off-by: Janakarajan Natarajan 

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 v3 4/8] xen_backend: add an emulation of grant copy

[Paul Durrant]

> -Original Message-
> From: Anthony PERARD [mailto:anthony.per...@citrix.com]
> Sent: 16 May 2018 15:31
> To: Paul Durrant 
> Cc: xen-devel@lists.xenproject.org; qemu-bl...@nongnu.org; qemu-
> de...@nongnu.org; Stefano Stabellini 
> Subject: Re: [PATCH v3 4/8] xen_backend: add an emulation of grant copy
> 
> On Fri, May 04, 2018 at 08:26:03PM +0100, Paul Durrant wrote:
> > Not all Xen environments support the xengnttab_grant_copy() operation.
> > E.g. where the OS is FreeBSD or Xen is older than 4.8.0.
> >
> > This patch introduces an emulation of that operation using
> > xengnttab_map_domain_grant_refs() and memcpy() for those
> environments.
> >
> > Signed-off-by: Paul Durrant 
> > ---
> 
> The patch looks ok, but since patch 1 is going to be change, this one is
> going to need to be change as well.
> 

Sure. Thanks for the review.

  Paul

> --
> Anthony PERARD

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

Re: [Xen-devel] [PATCH v3 4/8] xen_backend: add an emulation of grant copy

[Anthony PERARD]

On Fri, May 04, 2018 at 08:26:03PM +0100, Paul Durrant wrote:
> Not all Xen environments support the xengnttab_grant_copy() operation.
> E.g. where the OS is FreeBSD or Xen is older than 4.8.0.
> 
> This patch introduces an emulation of that operation using
> xengnttab_map_domain_grant_refs() and memcpy() for those environments.
> 
> Signed-off-by: Paul Durrant 
> ---

The patch looks ok, but since patch 1 is going to be change, this one is
going to need to be change as well.

-- 
Anthony PERARD

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

Re: [Xen-devel] [PATCH v3 3/8] xen: remove other open-coded use of libxengnttab

[Paul Durrant]

> -Original Message-
> From: Anthony PERARD [mailto:anthony.per...@citrix.com]
> Sent: 16 May 2018 15:14
> To: Paul Durrant 
> Cc: xen-devel@lists.xenproject.org; qemu-bl...@nongnu.org; qemu-
> de...@nongnu.org; Stefano Stabellini ; Greg Kurz
> ; Paolo Bonzini ; Jason Wang
> ; Gerd Hoffmann 
> Subject: Re: [PATCH v3 3/8] xen: remove other open-coded use of
> libxengnttab
> 
> On Fri, May 04, 2018 at 08:26:02PM +0100, Paul Durrant wrote:
> > Now that helpers are available in xen_backend, use them throughout all
> > Xen PV backends.
> >
> > Signed-off-by: Paul Durrant 
> > ---
> > diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c
> > index 20c43a6..73d6f1b 100644
> > --- a/hw/net/xen_nic.c
> > +++ b/hw/net/xen_nic.c
> > @@ -160,9 +160,8 @@ static void net_tx_packets(struct XenNetDev
> *netdev)
> >(txreq.flags & NETTXF_more_data)  ? " 
> > more_data"  : "",
> >(txreq.flags & NETTXF_extra_info) ? " 
> > extra_info" : "");
> >
> > -page = xengnttab_map_grant_ref(netdev->xendev.gnttabdev,
> > -   netdev->xendev.dom,
> > -   txreq.gref, PROT_READ);
> > +page = xen_be_map_grant_refs(>xendev,
> > + , 1, PROT_READ);
> 
> xen_be_map_grant_ref instead?
> 

Yep.

> >  if (page == NULL) {
> >  xen_pv_printf(>xendev, 0,
> >"error: tx gref dereference failed (%d)\n",
> > @@ -183,7 +182,7 @@ static void net_tx_packets(struct XenNetDev
> *netdev)
> >  qemu_send_packet(qemu_get_queue(netdev->nic),
> >   page + txreq.offset, txreq.size);
> >  }
> > -xengnttab_unmap(netdev->xendev.gnttabdev, page, 1);
> > +xen_be_unmap_grant_ref(>xendev, page);
> >  net_tx_response(netdev, , NETIF_RSP_OKAY);
> >  }
> >  if (!netdev->tx_work) {
> > @@ -254,9 +253,8 @@ static ssize_t net_rx_packet(NetClientState *nc,
> const uint8_t *buf, size_t size
> >  memcpy(, RING_GET_REQUEST(>rx_ring, rc),
> sizeof(rxreq));
> >  netdev->rx_ring.req_cons = ++rc;
> >
> > -page = xengnttab_map_grant_ref(netdev->xendev.gnttabdev,
> > -   netdev->xendev.dom,
> > -   rxreq.gref, PROT_WRITE);
> > +page = xen_be_map_grant_refs(>xendev, , 1,
> > + PROT_WRITE);
> 
> xen_be_map_grant_ref instead?
> 

And yep again.

> With that fix:
> Acked-by: Anthony PERARD 
> 

Thanks :-)

  Paul

> --
> Anthony PERARD

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

Re: [Xen-devel] [PATCH v3 2/8] xen_disk: remove open-coded use of libxengnttab

[Anthony PERARD]

On Fri, May 04, 2018 at 08:26:01PM +0100, Paul Durrant wrote:
> Now that helpers are present in xen_backend, this patch removes open-coded
> calls to libxengnttab from the xen_disk code.
> 
> This patch also fixes one whitspace error in the assignment of the
> XenDevOps initialise method.
> 
> Signed-off-by: Paul Durrant 

Acked-by: Anthony PERARD 

-- 
Anthony PERARD

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

Re: [Xen-devel] [PATCH v2 3/6] hvm/mtrr: use the hardware number of variable ranges for Dom0

[Roger Pau Monné]

On Wed, May 16, 2018 at 07:51:07AM -0600, Jan Beulich wrote:
> >>> On 16.05.18 at 15:41,  wrote:
> > On Wed, May 16, 2018 at 06:01:00AM -0600, Jan Beulich wrote:
> >> @@ -729,6 +729,14 @@ static int hvm_load_mtrr_msr(struct doma
> >>  if ( hvm_load_entry(MTRR, h, _mtrr) != 0 )
> >>  return -EINVAL;
> >>  
> >> +if ( (uint8_t)hw_mtrr.msr_mtrr_cap > MTRR_VCNT )
> > 
> > And here I would compare against the VCNT in mtrr_state->mtrr_cat
> > instead of MTRR_VCNT if you agree.
> 
> No, I don't agree: We're _defining_ the number of MTRRs for this vCPU
> here. There's no difference if such a record is loaded only once, or if all
> such records agree in count. But I don't see why we should refuse
> loading a record with a count higher than that provided by an earlier
> record, but no larger than MTRR_VCNT.

You are right, a previous load could have changed mtrr_state->mtrr_cap
to < MTRR_VCNT, and then subsequent intents to load MTRR_VCNT number
of MTRRs would fail.

Thanks, Roger.

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

Re: [Xen-devel] backporting considerations (Re: [PATCH v9 0/9] xen/x86: various XPTI speedups)

[Jan Beulich]

>>> On 16.05.18 at 15:18,  wrote:
> On Wed, May 16, 2018 at 10:06 AM, Jan Beulich  wrote:
> On 26.04.18 at 13:33,  wrote:
>>> Juergen Gross (9):
>>>   x86/xpti: avoid copying L4 page table contents when possible
>>>   xen/x86: add a function for modifying cr3
>>>   xen/x86: support per-domain flag for xpti
>>>   xen/x86: use invpcid for flushing the TLB
>>>   xen/x86: disable global pages for domains with XPTI active
>>>   xen/x86: use flag byte for decision whether xen_cr3 is valid
>>>   xen/x86: convert pv_guest_cr4_to_real_cr4() to a function
>>>   xen/x86: add some cr3 helpers
>>>   xen/x86: use PCID feature
>>
>> This being a performance improvement rather than a plain bug fix series,
>> I'm not entirely certain about backporting here. My current thinking is to
>> put this into 4.10 (Jürgen was kind enough to do the backporting work
>> already), but not into any older trees. Otoh at SUSE we already have
>> this in our 4.9-based branch as well, and if other consumers of that or
>> the 4.8 branch would mostly agree it should go there, I could certainly
>> be convinced.
> 
> Turning on XPTI causes a pretty large regression in performance; and
> these reduce that regression significantly.  I'm pretty sure that most
> downstreams will end up backporting these anyway (I'm sure CentOS
> will); it's much better to do it officially once, rather than have
> individual downstreams (most of whom do not have hypervisor developers
> maintaining packages) all do it separately.

Thanks, recorded as a data point. I don't, however, consider "do not
have hypervisor developers" as a valid excuse. Package maintainers
ought to be understanding their packages well enough to be capable of
doing such backports if they really think they need them. Basically this
goes back to there being (too many?) downstreams who don't care at
all to contribute anything back.

>> It is imo out of question of putting it into 4.7 or older.
> 
> Is that because of the complexity?  Or because 4.7 is in "security-only" 
> mode?

The latter.

> If the latter, I think the same argument applies: turning on XPTI is a
> requirement for many people, and thus represents a pretty hefty
> performance regression.  While we don't need to backport normal fixes
> to security-only releases, we should certainly try to avoid
> regressions.

I don't think we would have addressed non-security fallout (or other
than really severe regressions) from other security patches in the
past on security only branches. People caring about performance
should upgrade.

Jan


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

Re: [Xen-devel] [PATCH v3 1/8] xen_backend: add grant table helpers

[Paul Durrant]

> -Original Message-
> From: Anthony PERARD [mailto:anthony.per...@citrix.com]
> Sent: 16 May 2018 14:50
> To: Paul Durrant 
> Cc: xen-devel@lists.xenproject.org; qemu-bl...@nongnu.org; qemu-
> de...@nongnu.org; Stefano Stabellini 
> Subject: Re: [PATCH v3 1/8] xen_backend: add grant table helpers
> 
> On Fri, May 04, 2018 at 08:26:00PM +0100, Paul Durrant wrote:
> > This patch adds grant table helper functions to the xen_backend code to
> > localize error reporting and use of xen_domid.
> >
> > The patch also defers the call to xengnttab_open() until just before the
> > initialise method in XenDevOps is invoked. This method is responsible for
> > mapping the shared ring. No prior method requires access to the grant
> table.
> >
> > Signed-off-by: Paul Durrant 
> > ---
> > Cc: Stefano Stabellini 
> > Cc: Anthony Perard 
> >
> > v2:
> >  - New in v2
> > ---
> >  hw/xen/xen_backend.c | 123
> ++-
> >  include/hw/xen/xen_backend.h |  33 
> >  2 files changed, 144 insertions(+), 12 deletions(-)
> >
> > diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
> > index 7445b50..50412d6 100644
> > --- a/hw/xen/xen_backend.c
> > +++ b/hw/xen/xen_backend.c
> > @@ -106,6 +106,103 @@ int xen_be_set_state(struct XenDevice *xendev,
> enum xenbus_state state)
> >  return 0;
> >  }
> >
> > +void xen_be_set_max_grant_refs(struct XenDevice *xendev,
> > +   unsigned int nr_refs)
> 
> Is it fine to ignore error from set_max_grants and continue ? xen_disk.c
> seems to fail the initialisation if set_max_grants call fails. On the
> other end, xen-usb.c just keep going.
> 

I guess the upshot will be that a subsequent grant map would fail, so I think 
it should be sufficient to deal with the failure there. As you say it's use is 
inconsistent, and just plain missing in some cases.

> > +{
> > +assert(xendev->ops->flags & DEVOPS_FLAG_NEED_GNTDEV);
> > +
> > +if (xengnttab_set_max_grants(xendev->gnttabdev, nr_refs)) {
> > +xen_pv_printf(xendev, 0, "xengnttab_set_max_grants failed: %s\n",
> > +  strerror(errno));
> > +}
> > +}
> > +
> 
> > +int xen_be_copy_grant_refs(struct XenDevice *xendev,
> > +   bool to_domain,
> > +   XenGrantCopySegment segs[],
> > +   unsigned int nr_segs)
> > +{
> > +xengnttab_grant_copy_segment_t *xengnttab_segs;
> > +unsigned int i;
> > +int rc;
> > +
> > +assert(xendev->ops->flags & DEVOPS_FLAG_NEED_GNTDEV);
> > +
> > +xengnttab_segs = g_new0(xengnttab_grant_copy_segment_t,
> nr_segs);
> > +
> > +for (i = 0; i < nr_segs; i++) {
> > +XenGrantCopySegment *seg = [i];
> > +xengnttab_grant_copy_segment_t *xengnttab_seg =
> _segs[i];
> > +
> > +if (to_domain) {
> > +xengnttab_seg->flags = GNTCOPY_dest_gref;
> > +xengnttab_seg->dest.foreign.domid = xen_domid;
> > +xengnttab_seg->dest.foreign.ref = seg->dest.foreign.ref;
> > +xengnttab_seg->dest.foreign.offset = seg->dest.foreign.offset;
> > +xengnttab_seg->source.virt = seg->source.virt;
> > +} else {
> > +xengnttab_seg->flags = GNTCOPY_source_gref;
> > +xengnttab_seg->source.foreign.domid = xen_domid;
> > +xengnttab_seg->source.foreign.ref = seg->source.foreign.ref;
> > +xengnttab_seg->source.foreign.offset =
> > +seg->source.foreign.offset;
> > +xengnttab_seg->dest.virt = seg->dest.virt;
> > +}
> 
> That's not going to work because xengnttab_grant_copy_segment_t doesn't
> exist on Xen 4.7.

Ah, I'd missed the ifdef around that in xen_disk. I'll add it here.

Cheers,

  Paul

> 
> > +
> > +xengnttab_seg->len = seg->len;
> > +}
> > +
> > +rc = xengnttab_grant_copy(xendev->gnttabdev, nr_segs,
> xengnttab_segs);
> 
> Thanks,
> 
> --
> Anthony PERARD

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

[Xen-devel] [distros-debian-squeeze test] 74720: tolerable FAIL

[Platform Team regression test user]

flight 74720 distros-debian-squeeze real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/74720/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-i386-squeeze-netboot-pygrub 10 debian-di-install fail like 
74699
 test-amd64-i386-i386-squeeze-netboot-pygrub 10 debian-di-install fail like 
74699
 test-amd64-amd64-amd64-squeeze-netboot-pygrub 10 debian-di-install fail like 
74699
 test-amd64-i386-amd64-squeeze-netboot-pygrub 10 debian-di-install fail like 
74699

baseline version:
 flight   74699

jobs:
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-amd64-squeeze-netboot-pygrubfail
 test-amd64-i386-amd64-squeeze-netboot-pygrub fail
 test-amd64-amd64-i386-squeeze-netboot-pygrub fail
 test-amd64-i386-i386-squeeze-netboot-pygrub  fail



sg-report-flight on osstest.xs.citrite.net
logs: /home/osstest/logs
images: /home/osstest/images

Logs, config files, etc. are available at
http://osstest.xs.citrite.net/~osstest/testlogs/logs

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


Push not applicable.


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

Re: [Xen-devel] [PATCH v2 3/6] hvm/mtrr: use the hardware number of variable ranges for Dom0

[Jan Beulich]

>>> On 16.05.18 at 15:41,  wrote:
> On Wed, May 16, 2018 at 06:01:00AM -0600, Jan Beulich wrote:
>> --- unstable.orig/xen/arch/x86/hvm/mtrr.c
>> +++ unstable/xen/arch/x86/hvm/mtrr.c
>> @@ -676,22 +676,22 @@ int hvm_set_mem_pinned_cacheattr(struct
>>  
>>  static int hvm_save_mtrr_msr(struct domain *d, hvm_domain_context_t *h)
>>  {
>> -int i;
>>  struct vcpu *v;
>> -struct hvm_hw_mtrr hw_mtrr;
>> -struct mtrr_state *mtrr_state;
>> +
>>  /* save mtrr */
>>  for_each_vcpu(d, v)
>>  {
>> -mtrr_state = >arch.hvm_vcpu.mtrr;
>> +const struct mtrr_state *mtrr_state = >arch.hvm_vcpu.mtrr;
>> +struct hvm_hw_mtrr hw_mtrr = {
>> +.msr_mtrr_def_type = mtrr_state->def_type |
>> + (mtrr_state->enabled << 10),
>> +.msr_mtrr_cap  = mtrr_state->mtrr_cap,
>> +};
>> +unsigned int i;
>>  
>>  hvm_get_guest_pat(v, _mtrr.msr_pat_cr);
>>  
>> -hw_mtrr.msr_mtrr_def_type = mtrr_state->def_type
>> -| (mtrr_state->enabled << 10);
>> -hw_mtrr.msr_mtrr_cap = mtrr_state->mtrr_cap;
>> -
>> -for ( i = 0; i < MTRR_VCNT; i++ )
>> +for ( i = 0; i < (uint8_t)hw_mtrr.msr_mtrr_cap; i++ )
> 
> I've changed this to use MASK_EXTR instead (and switched the cases
> below).

Thanks, that was the intention anyway.

>> @@ -729,6 +729,14 @@ static int hvm_load_mtrr_msr(struct doma
>>  if ( hvm_load_entry(MTRR, h, _mtrr) != 0 )
>>  return -EINVAL;
>>  
>> +if ( (uint8_t)hw_mtrr.msr_mtrr_cap > MTRR_VCNT )
> 
> And here I would compare against the VCNT in mtrr_state->mtrr_cat
> instead of MTRR_VCNT if you agree.

No, I don't agree: We're _defining_ the number of MTRRs for this vCPU
here. There's no difference if such a record is loaded only once, or if all
such records agree in count. But I don't see why we should refuse
loading a record with a count higher than that provided by an earlier
record, but no larger than MTRR_VCNT.

Jan



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

Re: [Xen-devel] [PATCH v3 1/8] xen_backend: add grant table helpers

[Anthony PERARD]

On Fri, May 04, 2018 at 08:26:00PM +0100, Paul Durrant wrote:
> This patch adds grant table helper functions to the xen_backend code to
> localize error reporting and use of xen_domid.
> 
> The patch also defers the call to xengnttab_open() until just before the
> initialise method in XenDevOps is invoked. This method is responsible for
> mapping the shared ring. No prior method requires access to the grant table.
> 
> Signed-off-by: Paul Durrant 
> ---
> Cc: Stefano Stabellini 
> Cc: Anthony Perard 
> 
> v2:
>  - New in v2
> ---
>  hw/xen/xen_backend.c | 123 
> ++-
>  include/hw/xen/xen_backend.h |  33 
>  2 files changed, 144 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
> index 7445b50..50412d6 100644
> --- a/hw/xen/xen_backend.c
> +++ b/hw/xen/xen_backend.c
> @@ -106,6 +106,103 @@ int xen_be_set_state(struct XenDevice *xendev, enum 
> xenbus_state state)
>  return 0;
>  }
>  
> +void xen_be_set_max_grant_refs(struct XenDevice *xendev,
> +   unsigned int nr_refs)

Is it fine to ignore error from set_max_grants and continue ? xen_disk.c
seems to fail the initialisation if set_max_grants call fails. On the
other end, xen-usb.c just keep going.

> +{
> +assert(xendev->ops->flags & DEVOPS_FLAG_NEED_GNTDEV);
> +
> +if (xengnttab_set_max_grants(xendev->gnttabdev, nr_refs)) {
> +xen_pv_printf(xendev, 0, "xengnttab_set_max_grants failed: %s\n",
> +  strerror(errno));
> +}
> +}
> +

> +int xen_be_copy_grant_refs(struct XenDevice *xendev,
> +   bool to_domain,
> +   XenGrantCopySegment segs[],
> +   unsigned int nr_segs)
> +{
> +xengnttab_grant_copy_segment_t *xengnttab_segs;
> +unsigned int i;
> +int rc;
> +
> +assert(xendev->ops->flags & DEVOPS_FLAG_NEED_GNTDEV);
> +
> +xengnttab_segs = g_new0(xengnttab_grant_copy_segment_t, nr_segs);
> +
> +for (i = 0; i < nr_segs; i++) {
> +XenGrantCopySegment *seg = [i];
> +xengnttab_grant_copy_segment_t *xengnttab_seg = _segs[i];
> +
> +if (to_domain) {
> +xengnttab_seg->flags = GNTCOPY_dest_gref;
> +xengnttab_seg->dest.foreign.domid = xen_domid;
> +xengnttab_seg->dest.foreign.ref = seg->dest.foreign.ref;
> +xengnttab_seg->dest.foreign.offset = seg->dest.foreign.offset;
> +xengnttab_seg->source.virt = seg->source.virt;
> +} else {
> +xengnttab_seg->flags = GNTCOPY_source_gref;
> +xengnttab_seg->source.foreign.domid = xen_domid;
> +xengnttab_seg->source.foreign.ref = seg->source.foreign.ref;
> +xengnttab_seg->source.foreign.offset =
> +seg->source.foreign.offset;
> +xengnttab_seg->dest.virt = seg->dest.virt;
> +}

That's not going to work because xengnttab_grant_copy_segment_t doesn't
exist on Xen 4.7.

> +
> +xengnttab_seg->len = seg->len;
> +}
> +
> +rc = xengnttab_grant_copy(xendev->gnttabdev, nr_segs, xengnttab_segs);

Thanks,

-- 
Anthony PERARD

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

Re: [Xen-devel] [Patch v3 2/2] x86/microcode: Synchronize late microcode loading

[Jan Beulich]

>>> On 16.05.18 at 15:25,  wrote:
> On 16/05/18 14:10, Jan Beulich wrote:
>>> +static int do_microcode_update(void *_info)
>>> +{
>>> +struct microcode_info *info = _info;
>>> +unsigned int cpu = smp_processor_id();
>>> +int ret;
>>> +
>>> +ret = wait_for_cpus(>cpu_in, MICROCODE_DEFAULT_TIMEOUT);
>>> +if ( ret )
>>> +return ret;
>>> +
>>> +/*
>>> + * Logical threads which set the first bit in cpu_sibling_mask can do
>>> + * the update. Other sibling threads just await the completion of
>>> + * microcode update.
>>> + */
>>> +if ( !cpumask_test_and_set_cpu(
>>> +cpumask_first(per_cpu(cpu_sibling_mask, cpu)), 
>>> >cpus) )
>>> +ret = microcode_update_cpu(info->buffer, info->buffer_size);
>>> +/*
>>> + * Increase the wait timeout to a safe value here since we're 
>>> serializing
>>> + * the microcode update and that could take a while on a large number 
>>> of
>>> + * CPUs. And that is fine as the *actual* timeout will be determined by
>>> + * the last CPU finished updating and thus cut short
>>> + */
>>> +if ( wait_for_cpus(>cpu_out, MICROCODE_DEFAULT_TIMEOUT *
>>> +   nr_phys_cpus) )
>> I remain unconvinced that this is a safe thing to do on a huge system with
>> guests running (even Dom0 alone would seem risky enough). I continue to
>> hope for comments from others, in particular Andrew, here. At the very
>> least I think you should taint the hypervisor when making it here.
> 
> I see nothing in this patch which prevents a deadlock against the time
> calibration rendezvous.  It think its fine to pause the time calibration
> rendezvous while performing this update.

If there's a problem here, wouldn't that be a general one with
stop_machine()?

> Also, what is the purpose of serialising the updates while all pcpus are
> in rendezvous?  Surely at that point the best option is to initiate an
> update on all processors which don't have an online sibling thread with
> a lower thread id.

I've suggested that before.

Jan



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

Re: [Xen-devel] [PATCH v2 3/6] hvm/mtrr: use the hardware number of variable ranges for Dom0

[Roger Pau Monné]

On Wed, May 16, 2018 at 06:01:00AM -0600, Jan Beulich wrote:
> >>> On 16.05.18 at 13:53,  wrote:
> > On Wed, May 16, 2018 at 02:39:26AM -0600, Jan Beulich wrote:
> >> >>> On 15.05.18 at 16:36,  wrote:
> >> > +for ( i = 0; i < num_var_ranges; i++ )
> >> 
> >> Following your v1 I had already put together a patch to change just the
> >> save and load functions here, as the adjustments are necessary
> >> independent of the Dom0 aspect. Should num_var_ranges indeed be
> >> below MTRR_VCNT, there's an information leak here (of hypervisor stack
> >> data) without pre-initializing hw_mtrr. Here's the hunk from my patch, in
> >> case you care to re-use parts of it:
> >> 
> >> @@ -676,22 +676,22 @@ int hvm_set_mem_pinned_cacheattr(struct
> >>  
> >>  static int hvm_save_mtrr_msr(struct domain *d, hvm_domain_context_t *h)
> >>  {
> >> -int i;
> >>  struct vcpu *v;
> >> -struct hvm_hw_mtrr hw_mtrr;
> >> -struct mtrr_state *mtrr_state;
> >> +
> >>  /* save mtrr */
> >>  for_each_vcpu(d, v)
> >>  {
> >> -mtrr_state = >arch.hvm_vcpu.mtrr;
> >> +const struct mtrr_state *mtrr_state = >arch.hvm_vcpu.mtrr;
> >> +struct hvm_hw_mtrr hw_mtrr = {
> >> +.msr_mtrr_def_type = mtrr_state->def_type |
> >> + (mtrr_state->enabled << 10),
> >> +.msr_mtrr_cap  = mtrr_state->mtrr_cap,
> >> +};
> >> +unsigned int i;
> >>  
> >>  hvm_get_guest_pat(v, _mtrr.msr_pat_cr);
> >>  
> >> -hw_mtrr.msr_mtrr_def_type = mtrr_state->def_type
> >> -| (mtrr_state->enabled << 10);
> >> -hw_mtrr.msr_mtrr_cap = mtrr_state->mtrr_cap;
> >> -
> >> -for ( i = 0; i < MTRR_VCNT; i++ )
> >> +for ( i = 0; i < (uint8_t)hw_mtrr.msr_mtrr_cap; i++ )
> >>  {
> >>  /* save physbase */
> >>  hw_mtrr.msr_mtrr_var[i*2] =
> >> 
> >> (I didn't send it out yet as I'm generally of the opinion that prior to
> >> branching focus should be on the code to be released rather than
> >> the next following version.)
> > 
> > Would you be OK if I integrate this as a pre-patch to this one in my
> > series?
> 
> Sure, but then maybe better use the full one:

Thanks, I've added this to my MTRR series just after the switch to use
MASK_EXTR to get the VCNT.

> x86/HVM: improve MTRR load checks
> 
> We should not assume that the incoming set of values contains exactly
> MTRR_VCNT variable range MSRs. Permit a smaller amount and reject a
> bigger one. As a result the save path then also needs to no longer use
> a fixed upper bound, in turn requiring unused space in the save record
> to be zeroed up front.
> 
> Also slightly refine types where appropriate.
> 
> Signed-off-by: Jan Beulich 
> 
> --- unstable.orig/xen/arch/x86/hvm/mtrr.c
> +++ unstable/xen/arch/x86/hvm/mtrr.c
> @@ -676,22 +676,22 @@ int hvm_set_mem_pinned_cacheattr(struct
>  
>  static int hvm_save_mtrr_msr(struct domain *d, hvm_domain_context_t *h)
>  {
> -int i;
>  struct vcpu *v;
> -struct hvm_hw_mtrr hw_mtrr;
> -struct mtrr_state *mtrr_state;
> +
>  /* save mtrr */
>  for_each_vcpu(d, v)
>  {
> -mtrr_state = >arch.hvm_vcpu.mtrr;
> +const struct mtrr_state *mtrr_state = >arch.hvm_vcpu.mtrr;
> +struct hvm_hw_mtrr hw_mtrr = {
> +.msr_mtrr_def_type = mtrr_state->def_type |
> + (mtrr_state->enabled << 10),
> +.msr_mtrr_cap  = mtrr_state->mtrr_cap,
> +};
> +unsigned int i;
>  
>  hvm_get_guest_pat(v, _mtrr.msr_pat_cr);
>  
> -hw_mtrr.msr_mtrr_def_type = mtrr_state->def_type
> -| (mtrr_state->enabled << 10);
> -hw_mtrr.msr_mtrr_cap = mtrr_state->mtrr_cap;
> -
> -for ( i = 0; i < MTRR_VCNT; i++ )
> +for ( i = 0; i < (uint8_t)hw_mtrr.msr_mtrr_cap; i++ )

I've changed this to use MASK_EXTR instead (and switched the cases
below).

>  {
>  /* save physbase */
>  hw_mtrr.msr_mtrr_var[i*2] =
> @@ -729,6 +729,14 @@ static int hvm_load_mtrr_msr(struct doma
>  if ( hvm_load_entry(MTRR, h, _mtrr) != 0 )
>  return -EINVAL;
>  
> +if ( (uint8_t)hw_mtrr.msr_mtrr_cap > MTRR_VCNT )

And here I would compare against the VCNT in mtrr_state->mtrr_cat
instead of MTRR_VCNT if you agree.

Thanks, Roger.

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

Re: [Xen-devel] [Patch v3 2/2] x86/microcode: Synchronize late microcode loading

[Andrew Cooper]

On 16/05/18 14:10, Jan Beulich wrote:
>> +static int do_microcode_update(void *_info)
>> +{
>> +struct microcode_info *info = _info;
>> +unsigned int cpu = smp_processor_id();
>> +int ret;
>> +
>> +ret = wait_for_cpus(>cpu_in, MICROCODE_DEFAULT_TIMEOUT);
>> +if ( ret )
>> +return ret;
>> +
>> +/*
>> + * Logical threads which set the first bit in cpu_sibling_mask can do
>> + * the update. Other sibling threads just await the completion of
>> + * microcode update.
>> + */
>> +if ( !cpumask_test_and_set_cpu(
>> +cpumask_first(per_cpu(cpu_sibling_mask, cpu)), >cpus) 
>> )
>> +ret = microcode_update_cpu(info->buffer, info->buffer_size);
>> +/*
>> + * Increase the wait timeout to a safe value here since we're 
>> serializing
>> + * the microcode update and that could take a while on a large number of
>> + * CPUs. And that is fine as the *actual* timeout will be determined by
>> + * the last CPU finished updating and thus cut short
>> + */
>> +if ( wait_for_cpus(>cpu_out, MICROCODE_DEFAULT_TIMEOUT *
>> +   nr_phys_cpus) )
> I remain unconvinced that this is a safe thing to do on a huge system with
> guests running (even Dom0 alone would seem risky enough). I continue to
> hope for comments from others, in particular Andrew, here. At the very
> least I think you should taint the hypervisor when making it here.

I see nothing in this patch which prevents a deadlock against the time
calibration rendezvous.  It think its fine to pause the time calibration
rendezvous while performing this update.

Also, what is the purpose of serialising the updates while all pcpus are
in rendezvous?  Surely at that point the best option is to initiate an
update on all processors which don't have an online sibling thread with
a lower thread id.

~Andrew

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

Re: [Xen-devel] backporting considerations (Re: [PATCH v9 0/9] xen/x86: various XPTI speedups)

[George Dunlap]

On Wed, May 16, 2018 at 10:06 AM, Jan Beulich  wrote:
 On 26.04.18 at 13:33,  wrote:
>> Juergen Gross (9):
>>   x86/xpti: avoid copying L4 page table contents when possible
>>   xen/x86: add a function for modifying cr3
>>   xen/x86: support per-domain flag for xpti
>>   xen/x86: use invpcid for flushing the TLB
>>   xen/x86: disable global pages for domains with XPTI active
>>   xen/x86: use flag byte for decision whether xen_cr3 is valid
>>   xen/x86: convert pv_guest_cr4_to_real_cr4() to a function
>>   xen/x86: add some cr3 helpers
>>   xen/x86: use PCID feature
>
> This being a performance improvement rather than a plain bug fix series,
> I'm not entirely certain about backporting here. My current thinking is to
> put this into 4.10 (Jürgen was kind enough to do the backporting work
> already), but not into any older trees. Otoh at SUSE we already have
> this in our 4.9-based branch as well, and if other consumers of that or
> the 4.8 branch would mostly agree it should go there, I could certainly
> be convinced.

Turning on XPTI causes a pretty large regression in performance; and
these reduce that regression significantly.  I'm pretty sure that most
downstreams will end up backporting these anyway (I'm sure CentOS
will); it's much better to do it officially once, rather than have
individual downstreams (most of whom do not have hypervisor developers
maintaining packages) all do it separately.

> It is imo out of question of putting it into 4.7 or older.

Is that because of the complexity?  Or because 4.7 is in "security-only" mode?

If the latter, I think the same argument applies: turning on XPTI is a
requirement for many people, and thus represents a pretty hefty
performance regression.  While we don't need to backport normal fixes
to security-only releases, we should certainly try to avoid
regressions.

(Note that this doesn't mean you personally have to do the work if you
think it's not worth your time.)

 -George

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

[Xen-devel] [xen-4.6-testing baseline-only test] 74719: regressions - FAIL

[Platform Team regression test user]

This run is configured for baseline tests only.

flight 74719 xen-4.6-testing real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/74719/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-xtf-amd64-amd64-521 xtf/test-hvm32-invlpg~shadow fail REGR. vs. 74647
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-installfail REGR. vs. 74647
 test-xtf-amd64-amd64-5 36 xtf/test-hvm32pae-invlpg~shadow fail REGR. vs. 74647
 test-xtf-amd64-amd64-549 xtf/test-hvm64-invlpg~shadow fail REGR. vs. 74647
 test-amd64-i386-xl-qemuu-ws16-amd64 10 windows-installfail REGR. vs. 74647
 test-amd64-i386-xl-qemuu-ovmf-amd64 21 leak-check/check   fail REGR. vs. 74647

Tests which did not succeed, but are not blocking:
 test-xtf-amd64-amd64-4   21 xtf/test-hvm32-invlpg~shadow fail   like 74647
 test-xtf-amd64-amd64-4  36 xtf/test-hvm32pae-invlpg~shadow fail like 74647
 test-xtf-amd64-amd64-4   49 xtf/test-hvm64-invlpg~shadow fail   like 74647
 test-armhf-armhf-libvirt-xsm 12 guest-start  fail   like 74647
 test-armhf-armhf-libvirt 12 guest-start  fail   like 74647
 test-armhf-armhf-xl-xsm  12 guest-start  fail   like 74647
 test-armhf-armhf-xl-multivcpu 12 guest-start  fail  like 74647
 test-armhf-armhf-xl-midway   12 guest-start  fail   like 74647
 test-armhf-armhf-xl-credit2  12 guest-start  fail   like 74647
 test-armhf-armhf-xl  12 guest-start  fail   like 74647
 test-armhf-armhf-xl-rtds 12 guest-start  fail   like 74647
 test-amd64-amd64-qemuu-nested-intel 14 xen-boot/l1 fail like 74647
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail like 74647
 test-armhf-armhf-xl-vhd  10 debian-di-installfail   like 74647
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail like 74647
 test-armhf-armhf-libvirt-raw 10 debian-di-installfail   like 74647
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail like 74647
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail like 74647
 test-xtf-amd64-amd64-2   37 xtf/test-hvm32pae-memop-seg  fail   never pass
 test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass
 test-xtf-amd64-amd64-2   52 xtf/test-hvm64-memop-seg fail   never pass
 test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass
 test-xtf-amd64-amd64-2   76 xtf/test-pv32pae-xsa-194 fail   never pass
 test-xtf-amd64-amd64-1   37 xtf/test-hvm32pae-memop-seg  fail   never pass
 test-xtf-amd64-amd64-3   37 xtf/test-hvm32pae-memop-seg  fail   never pass
 test-xtf-amd64-amd64-1   52 xtf/test-hvm64-memop-seg fail   never pass
 test-amd64-amd64-xl-qemut-ws16-amd64 10 windows-installfail never pass
 test-xtf-amd64-amd64-3   52 xtf/test-hvm64-memop-seg fail   never pass
 test-xtf-amd64-amd64-4   37 xtf/test-hvm32pae-memop-seg  fail   never pass
 test-xtf-amd64-amd64-5   37 xtf/test-hvm32pae-memop-seg  fail   never pass
 test-xtf-amd64-amd64-1   76 xtf/test-pv32pae-xsa-194 fail   never pass
 test-xtf-amd64-amd64-3   76 xtf/test-pv32pae-xsa-194 fail   never pass
 test-xtf-amd64-amd64-4   52 xtf/test-hvm64-memop-seg fail   never pass
 test-xtf-amd64-amd64-5   52 xtf/test-hvm64-memop-seg fail   never pass
 test-xtf-amd64-amd64-4   76 xtf/test-pv32pae-xsa-194 fail   never pass
 test-xtf-amd64-amd64-5   76 xtf/test-pv32pae-xsa-194 fail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-installfail never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 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-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-amd64-amd64-xl-qemuu-win10-i386 17 guest-stop fail never pass
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop  fail never pass

version targeted for testing:
 xen  6a74f4e31dc28fb0d5c9e56b54d4b2aaf9b46bbe
baseline version:
 xen  927aca70011f83c44294f90275c18a0b3f7d7169

Last test of basis74647  2018-04-28 22:47:09 Z   17 days
Testing same since74719  2018-05-16 01:17:41 Z0 days1 attempts


People who touched revisions under test:
  Andrew 

Re: [Xen-devel] [Patch v3 2/2] x86/microcode: Synchronize late microcode loading

[Jan Beulich]

>>> On 09.05.18 at 00:01,  wrote:
> @@ -30,15 +31,21 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> +#include 
>  
> +#include 
>  #include 
>  #include 
>  #include 
>  #include 
>  
> +/* By default, wait for 3us */
> +#define MICROCODE_DEFAULT_TIMEOUT 3

Please attach the unit (_US) to the name.

> @@ -281,24 +288,56 @@ static int microcode_update_cpu(const void *buf, size_t 
> size)
>  return err;
>  }
>  
> -static long do_microcode_update(void *_info)
> +/* Wait for all CPUs to rendezvous with a timeout (us) */
> +static int wait_for_cpus(atomic_t *cnt, unsigned int timeout)
>  {
> -struct microcode_info *info = _info;
> -int error;
> +unsigned int cpus = num_online_cpus();
>  
> -BUG_ON(info->cpu != smp_processor_id());
> +atomic_inc(cnt);
>  
> -error = microcode_update_cpu(info->buffer, info->buffer_size);
> -if ( error )
> -info->error = error;
> +while ( atomic_read(cnt) != cpus )
> +{
> +if ( timeout <= 0 )

!timeout (or timeout == 0), now that it's of unsigned type.

> +static int do_microcode_update(void *_info)
> +{
> +struct microcode_info *info = _info;
> +unsigned int cpu = smp_processor_id();
> +int ret;
> +
> +ret = wait_for_cpus(>cpu_in, MICROCODE_DEFAULT_TIMEOUT);
> +if ( ret )
> +return ret;
> +
> +/*
> + * Logical threads which set the first bit in cpu_sibling_mask can do
> + * the update. Other sibling threads just await the completion of
> + * microcode update.
> + */
> +if ( !cpumask_test_and_set_cpu(
> +cpumask_first(per_cpu(cpu_sibling_mask, cpu)), >cpus) )
> +ret = microcode_update_cpu(info->buffer, info->buffer_size);
> +/*
> + * Increase the wait timeout to a safe value here since we're serializing
> + * the microcode update and that could take a while on a large number of
> + * CPUs. And that is fine as the *actual* timeout will be determined by
> + * the last CPU finished updating and thus cut short
> + */
> +if ( wait_for_cpus(>cpu_out, MICROCODE_DEFAULT_TIMEOUT *
> +   nr_phys_cpus) )

I remain unconvinced that this is a safe thing to do on a huge system with
guests running (even Dom0 alone would seem risky enough). I continue to
hope for comments from others, in particular Andrew, here. At the very
least I think you should taint the hypervisor when making it here.

> +/*
> + * Late loading dance. Why the heavy-handed stop_machine effort?
> + *
> + * -HT siblings must be idle and not execute other code while the other
> + *  sibling is loading microcode in order to avoid any negative
> + *  interactions cause by the loading.
> + *
> + * -In addition, microcode update on the cores must be serialized until
> + *  this requirement can be relaxed in the feature. Right now, this is

s/feature/future/

Also please add blanks after the hyphens.

Jan



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

[Xen-devel] [xen-4.9-testing test] 122785: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-arm64-arm64-xl  broken  in 122710
 test-arm64-arm64-libvirt-xsm broken  in 122710
 test-arm64-arm64-xl-credit2  broken  in 122710
 test-arm64-arm64-xl-xsm  broken  in 122710
 test-amd64-amd64-xl-qemut-ws16-amd64 16 guest-localmigrate/x10 fail in 122659 
REGR. vs. 122512

Tests which are failing intermittently (not blocking):
 test-arm64-arm64-libvirt-xsm 4 host-install(4) broken in 122710 pass in 122785
 test-arm64-arm64-xl-xsm  4 host-install(4) broken in 122710 pass in 122785
 test-arm64-arm64-xl-credit2  4 host-install(4) broken in 122710 pass in 122785
 test-arm64-arm64-xl  4 host-install(4) broken in 122710 pass in 122785
 test-amd64-amd64-xl-credit2  12 guest-start  fail in 122659 pass in 122785
 test-amd64-amd64-xl  12 guest-start  fail in 122659 pass in 122785
 test-amd64-i386-pair   21 guest-start/debian fail in 122659 pass in 122785
 test-amd64-amd64-i386-pvgrub 11 guest-start  fail in 122659 pass in 122785
 test-amd64-amd64-xl-multivcpu 20 guest-start/debian.repeat fail in 122659 pass 
in 122785
 test-armhf-armhf-xl-xsm  12 guest-start  fail in 122659 pass in 122785
 test-amd64-i386-xl-qemut-debianhvm-amd64 16 guest-localmigrate/x10 fail in 
122659 pass in 122785
 test-armhf-armhf-xl-arndale  17 guest-start.2fail in 122659 pass in 122785
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 18 
guest-start/debianhvm.repeat fail in 122659 pass in 122785
 test-amd64-amd64-amd64-pvgrub 20 guest-start.2   fail in 122659 pass in 122785
 test-armhf-armhf-xl-credit2  12 guest-start  fail in 122659 pass in 122785
 test-armhf-armhf-libvirt-xsm 12 guest-start  fail in 122659 pass in 122785
 test-amd64-i386-xl-qemut-ws16-amd64 16 guest-localmigrate/x10 fail in 122659 
pass in 122785
 test-armhf-armhf-xl-cubietruck 16 guest-start/debian.repeat fail in 122659 
pass in 122785
 test-armhf-armhf-xl-multivcpu 17 guest-start.2   fail in 122659 pass in 122785
 test-armhf-armhf-libvirt 16 guest-start/debian.repeat fail in 122659 pass in 
122785
 test-amd64-amd64-xl-qemut-ws16-amd64 15 guest-saverestore.2 fail in 122710 
pass in 122659
 test-amd64-amd64-xl-qemuu-ovmf-amd64 16 guest-localmigrate/x10 fail pass in 
122659
 test-armhf-armhf-xl   7 xen-boot   fail pass in 122710
 test-amd64-amd64-xl-qemut-ws16-amd64 14 guest-localmigrate fail pass in 122710
 test-amd64-amd64-rumprun-amd64 17 rumprun-demo-xenstorels/xenstorels.repeat 
fail pass in 122710
 test-amd64-i386-xl-qemut-debianhvm-amd64 17 guest-stop fail pass in 122710

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop  fail blocked in 122512
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-localmigrate/x10 fail in 122659 
like 122472
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 16 guest-localmigrate/x10 fail in 
122659 like 122512
 test-amd64-amd64-xl-qemuu-ws16-amd64 16 guest-localmigrate/x10 fail in 122710 
blocked in 122512
 test-amd64-i386-xl-qemuu-ws16-amd64 18 guest-start/win.repeat fail in 122710 
like 122417
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-localmigrate/x10 fail in 122710 
like 122472
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop   fail in 122710 like 122512
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-localmigrate/x10 fail in 122710 
like 122512
 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeat fail in 122710 like 
122512
 test-armhf-armhf-xl-rtds13 migrate-support-check fail in 122710 never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-check fail in 122710 never pass
 test-armhf-armhf-xl 13 migrate-support-check fail in 122710 never pass
 test-armhf-armhf-xl 14 saverestore-support-check fail in 122710 never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 122417
 test-armhf-armhf-xl-rtds 12 guest-start  fail  like 122472
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-localmigrate/x10 fail like 122472
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 122472
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail like 122512
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 122512
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 122512
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   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-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 

Re: [Xen-devel] Clarification Meltdown for vulnerability between 64-bit PV guests

[Andrew Cooper]

On 16/05/18 09:37, John Wang wrote:
>
> Hi:
>
> 64 bit PV guest can attack hypervisor by SP3
>

Yes.

> whether it still can attack others 64 bit PV guest by SP3?
>

Meltdown attacks only operate within a single address space.  You can't
attack a separate address space with it.

That said, the VM => VM attack with Meltdown is due to the fact that
other VMs are mapped into Xen's directmap, so available by attacking the
hypervisor.

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

Re: [Xen-devel] [Patch v3 1/2] x86/smp: count the number of online physical processor in the system

[Jan Beulich]

>>> On 09.05.18 at 00:01,  wrote:
> Mainly for the patch behind which relies on 'nr_phys_cpus' to estimate
> the time needed for microcode update in the worst case.

Leaving aside the aspect of "nr_phys_cpus" not being a suitable name
("nr_online_cores" would come closer imo) I'm not convinced using a
global variable is the way to go here, with just that special purpose
consumer in the next patch. Why can't you work out this count right
there?

Jan



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

Re: [Xen-devel] [PATCH v2 3/3] vpci/msi: fix update of bound MSI interrupts

[Jan Beulich]

>>> On 15.05.18 at 16:10,  wrote:
> Current update process of already bound MSI interrupts is wrong
> because unmap_domain_pirq calls pci_disable_msi, which disables MSI
> interrupts on the device. On the other hand map_domain_pirq doesn't
> enable MSI, so the current update process of already enabled MSI
> entries is wrong because MSI control bit will be disabled by
> unmap_domain_pirq and not re-enabled by map_domain_pirq.
> 
> In order to fix this avoid unmapping the PIRQs and just update the
> binding of the PIRQ. A new arch helper to do that is introduced.
> 
> Note that MSI-X is not affected because unmap_domain_pirq only
> disables the MSI enable control bit for the MSI case, for MSI-X the
> bit is left untouched by unmap_domain_pirq.
> 
> 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 v2 2/3] vpci/msi: split code to bind pirq

[Jan Beulich]

>>> On 15.05.18 at 16:10,  wrote:
> And put it in a separate update function. This is required in order to
> improve binding of MSI PIRQs when using vPCI.
> 
> No functional change.
> 
> 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 v2 1/3] vpci/msi: fix unbind loop

[Jan Beulich]

>>> On 15.05.18 at 16:10,  wrote:
> The current unbind loop on failure in vpci_msi_enable is wrong and
> will only work correctly if the initial pirq is 0. Fix this by adding
> a proper bound.
> 
> Reported-by: Jan Beulich 
> Signed-off-by: Roger Pau Monné 

Reviewed-by: Jan Beulich 

Jürgen,

any thoughts on taking this one for 4.11?

Jan

> --- a/xen/arch/x86/hvm/vmsi.c
> +++ b/xen/arch/x86/hvm/vmsi.c
> @@ -710,7 +710,7 @@ static int vpci_msi_enable(const struct pci_dev *pdev, 
> uint32_t data,
>   "%04x:%02x:%02x.%u: failed to bind PIRQ %u: %d\n",
>   pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
>   PCI_FUNC(pdev->devfn), pirq + i, rc);
> -while ( bind.machine_irq-- )
> +while ( bind.machine_irq-- > pirq )
>  pt_irq_destroy_bind(pdev->domain, );
>  spin_lock(>domain->event_lock);
>  unmap_domain_pirq(pdev->domain, pirq);
> -- 
> 2.17.0




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

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

[osstest service owner]

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

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  2adc90908fbb1e614c477e29f2d45eda94570795
baseline version:
 xen  b953322c5772dbc537421f9e2f97026a1c2fcb2e

Last test of basis   122852  2018-05-15 18:00:46 Z0 days
Testing same since   122868  2018-05-16 10:00:53 Z0 days1 attempts


People who touched revisions under test:
  Christian Lindig 
  John Thomson 

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



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

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

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

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


Pushing revision :

To xenbits.xen.org:/home/xen/git/xen.git
   b953322c57..2adc90908f  2adc90908fbb1e614c477e29f2d45eda94570795 -> smoke

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

Re: [Xen-devel] [PATCH v2 4/6] hvm/mtrr: copy hardware state for Dom0

[Jan Beulich]

>>> On 16.05.18 at 13:58,  wrote:
> On Wed, May 16, 2018 at 02:47:39AM -0600, Jan Beulich wrote:
>> >>> On 15.05.18 at 16:36,  wrote:
>> > --- a/xen/arch/x86/hvm/mtrr.c
>> > +++ b/xen/arch/x86/hvm/mtrr.c
>> > @@ -185,6 +185,30 @@ int hvm_vcpu_cacheattr_init(struct vcpu *v)
>> >  ((uint64_t)PAT_TYPE_UC_MINUS << 48) |   /* PAT6: UC- */
>> >  ((uint64_t)PAT_TYPE_UNCACHABLE << 56);  /* PAT7: UC */
>> >  
>> > +if ( is_hardware_domain(v->domain) )
>> > +{
>> > +/* Copy values from the host. */
>> > +struct domain *d = v->domain;
>> > +unsigned int i;
>> > +
>> > +if ( mtrr_state.have_fixed )
>> > +for ( i = 0; i < NUM_FIXED_MSR; i++ )
>> > +mtrr_fix_range_msr_set(d, m, i,
>> > +  ((uint64_t 
>> > *)mtrr_state.fixed_ranges)[i]);
>> > +
>> > +for ( i = 0; i < num_var_ranges; i++ )
>> > +{
>> > +mtrr_var_range_msr_set(d, m, MSR_IA32_MTRR_PHYSBASE(i),
>> > +   mtrr_state.var_ranges[i].base);
>> > +mtrr_var_range_msr_set(d, m, MSR_IA32_MTRR_PHYSMASK(i),
>> > +   mtrr_state.var_ranges[i].mask);
>> > +}
>> > +
>> > +mtrr_def_type_msr_set(d, m,
>> > +  mtrr_state.def_type |
>> > +  (mtrr_state.enabled << 
>> > MTRRdefType_FE_SHIFT));
>> 
>> This is all very clumsy (not your fault of course) - in particular, the 
>> "enabled"
>> field is a two-bit value. I'd rather not see this to continue to be that way,
>> and hence I've created a patch to clean this up (see below; I'm intentionally
>> retaining my own TODO notes in there for your reference, and it's also
>> unlikely to apply as is because it sits on top of other changes). In that 
>> light I
>> think I'd prefer if you either (later) re-based this onto my patch or 
> reverted
>> back to the use of the literal 10 here.
> 
> I've also realized this but thought about fixing it later.
> 
> I don't mind adding this patch to my MTRR series in order to make
> rebasing easier on my side if it's OK for you.

Feel free.

> I would likely add the enabled and fixed_enabled masks to msr-index.h
> and use MASK_* instead of shifts while there.

Right, that is the net effect of one of the two TODOs I have in there.

Jan



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

Re: [Xen-devel] [PATCH v2 3/6] hvm/mtrr: use the hardware number of variable ranges for Dom0

[Jan Beulich]

>>> On 16.05.18 at 13:53,  wrote:
> On Wed, May 16, 2018 at 02:39:26AM -0600, Jan Beulich wrote:
>> >>> On 15.05.18 at 16:36,  wrote:
>> > +for ( i = 0; i < num_var_ranges; i++ )
>> 
>> Following your v1 I had already put together a patch to change just the
>> save and load functions here, as the adjustments are necessary
>> independent of the Dom0 aspect. Should num_var_ranges indeed be
>> below MTRR_VCNT, there's an information leak here (of hypervisor stack
>> data) without pre-initializing hw_mtrr. Here's the hunk from my patch, in
>> case you care to re-use parts of it:
>> 
>> @@ -676,22 +676,22 @@ int hvm_set_mem_pinned_cacheattr(struct
>>  
>>  static int hvm_save_mtrr_msr(struct domain *d, hvm_domain_context_t *h)
>>  {
>> -int i;
>>  struct vcpu *v;
>> -struct hvm_hw_mtrr hw_mtrr;
>> -struct mtrr_state *mtrr_state;
>> +
>>  /* save mtrr */
>>  for_each_vcpu(d, v)
>>  {
>> -mtrr_state = >arch.hvm_vcpu.mtrr;
>> +const struct mtrr_state *mtrr_state = >arch.hvm_vcpu.mtrr;
>> +struct hvm_hw_mtrr hw_mtrr = {
>> +.msr_mtrr_def_type = mtrr_state->def_type |
>> + (mtrr_state->enabled << 10),
>> +.msr_mtrr_cap  = mtrr_state->mtrr_cap,
>> +};
>> +unsigned int i;
>>  
>>  hvm_get_guest_pat(v, _mtrr.msr_pat_cr);
>>  
>> -hw_mtrr.msr_mtrr_def_type = mtrr_state->def_type
>> -| (mtrr_state->enabled << 10);
>> -hw_mtrr.msr_mtrr_cap = mtrr_state->mtrr_cap;
>> -
>> -for ( i = 0; i < MTRR_VCNT; i++ )
>> +for ( i = 0; i < (uint8_t)hw_mtrr.msr_mtrr_cap; i++ )
>>  {
>>  /* save physbase */
>>  hw_mtrr.msr_mtrr_var[i*2] =
>> 
>> (I didn't send it out yet as I'm generally of the opinion that prior to
>> branching focus should be on the code to be released rather than
>> the next following version.)
> 
> Would you be OK if I integrate this as a pre-patch to this one in my
> series?

Sure, but then maybe better use the full one:

x86/HVM: improve MTRR load checks

We should not assume that the incoming set of values contains exactly
MTRR_VCNT variable range MSRs. Permit a smaller amount and reject a
bigger one. As a result the save path then also needs to no longer use
a fixed upper bound, in turn requiring unused space in the save record
to be zeroed up front.

Also slightly refine types where appropriate.

Signed-off-by: Jan Beulich 

--- unstable.orig/xen/arch/x86/hvm/mtrr.c
+++ unstable/xen/arch/x86/hvm/mtrr.c
@@ -676,22 +676,22 @@ int hvm_set_mem_pinned_cacheattr(struct
 
 static int hvm_save_mtrr_msr(struct domain *d, hvm_domain_context_t *h)
 {
-int i;
 struct vcpu *v;
-struct hvm_hw_mtrr hw_mtrr;
-struct mtrr_state *mtrr_state;
+
 /* save mtrr */
 for_each_vcpu(d, v)
 {
-mtrr_state = >arch.hvm_vcpu.mtrr;
+const struct mtrr_state *mtrr_state = >arch.hvm_vcpu.mtrr;
+struct hvm_hw_mtrr hw_mtrr = {
+.msr_mtrr_def_type = mtrr_state->def_type |
+ (mtrr_state->enabled << 10),
+.msr_mtrr_cap  = mtrr_state->mtrr_cap,
+};
+unsigned int i;
 
 hvm_get_guest_pat(v, _mtrr.msr_pat_cr);
 
-hw_mtrr.msr_mtrr_def_type = mtrr_state->def_type
-| (mtrr_state->enabled << 10);
-hw_mtrr.msr_mtrr_cap = mtrr_state->mtrr_cap;
-
-for ( i = 0; i < MTRR_VCNT; i++ )
+for ( i = 0; i < (uint8_t)hw_mtrr.msr_mtrr_cap; i++ )
 {
 /* save physbase */
 hw_mtrr.msr_mtrr_var[i*2] =
@@ -729,6 +729,14 @@ static int hvm_load_mtrr_msr(struct doma
 if ( hvm_load_entry(MTRR, h, _mtrr) != 0 )
 return -EINVAL;
 
+if ( (uint8_t)hw_mtrr.msr_mtrr_cap > MTRR_VCNT )
+{
+dprintk(XENLOG_G_ERR,
+"HVM restore: %pv: too many (%d) variable range MTRRs\n",
+v, (uint8_t)hw_mtrr.msr_mtrr_cap);
+return -EINVAL;
+}
+
 mtrr_state = >arch.hvm_vcpu.mtrr;
 
 hvm_set_guest_pat(v, hw_mtrr.msr_pat_cr);
@@ -738,7 +746,7 @@ static int hvm_load_mtrr_msr(struct doma
 for ( i = 0; i < NUM_FIXED_MSR; i++ )
 mtrr_fix_range_msr_set(d, mtrr_state, i, hw_mtrr.msr_mtrr_fixed[i]);
 
-for ( i = 0; i < MTRR_VCNT; i++ )
+for ( i = 0; i < (uint8_t)hw_mtrr.msr_mtrr_cap; i++ )
 {
 mtrr_var_range_msr_set(d, mtrr_state,
MSR_IA32_MTRR_PHYSBASE(i),

Jan


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

Re: [Xen-devel] [PATCH v2 4/6] hvm/mtrr: copy hardware state for Dom0

[Roger Pau Monné]

On Wed, May 16, 2018 at 02:47:39AM -0600, Jan Beulich wrote:
> >>> On 15.05.18 at 16:36,  wrote:
> > --- a/xen/arch/x86/hvm/mtrr.c
> > +++ b/xen/arch/x86/hvm/mtrr.c
> > @@ -185,6 +185,30 @@ int hvm_vcpu_cacheattr_init(struct vcpu *v)
> >  ((uint64_t)PAT_TYPE_UC_MINUS << 48) |   /* PAT6: UC- */
> >  ((uint64_t)PAT_TYPE_UNCACHABLE << 56);  /* PAT7: UC */
> >  
> > +if ( is_hardware_domain(v->domain) )
> > +{
> > +/* Copy values from the host. */
> > +struct domain *d = v->domain;
> > +unsigned int i;
> > +
> > +if ( mtrr_state.have_fixed )
> > +for ( i = 0; i < NUM_FIXED_MSR; i++ )
> > +mtrr_fix_range_msr_set(d, m, i,
> > +  ((uint64_t 
> > *)mtrr_state.fixed_ranges)[i]);
> > +
> > +for ( i = 0; i < num_var_ranges; i++ )
> > +{
> > +mtrr_var_range_msr_set(d, m, MSR_IA32_MTRR_PHYSBASE(i),
> > +   mtrr_state.var_ranges[i].base);
> > +mtrr_var_range_msr_set(d, m, MSR_IA32_MTRR_PHYSMASK(i),
> > +   mtrr_state.var_ranges[i].mask);
> > +}
> > +
> > +mtrr_def_type_msr_set(d, m,
> > +  mtrr_state.def_type |
> > +  (mtrr_state.enabled << 
> > MTRRdefType_FE_SHIFT));
> 
> This is all very clumsy (not your fault of course) - in particular, the 
> "enabled"
> field is a two-bit value. I'd rather not see this to continue to be that way,
> and hence I've created a patch to clean this up (see below; I'm intentionally
> retaining my own TODO notes in there for your reference, and it's also
> unlikely to apply as is because it sits on top of other changes). In that 
> light I
> think I'd prefer if you either (later) re-based this onto my patch or reverted
> back to the use of the literal 10 here.

I've also realized this but thought about fixing it later.

I don't mind adding this patch to my MTRR series in order to make
rebasing easier on my side if it's OK for you.

I would likely add the enabled and fixed_enabled masks to msr-index.h
and use MASK_* instead of shifts while there.

Thanks, Roger.

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

Re: [Xen-devel] [PATCH v2 3/6] hvm/mtrr: use the hardware number of variable ranges for Dom0

[Roger Pau Monné]

On Wed, May 16, 2018 at 02:39:26AM -0600, Jan Beulich wrote:
> >>> On 15.05.18 at 16:36,  wrote:
> > +for ( i = 0; i < num_var_ranges; i++ )
> 
> Following your v1 I had already put together a patch to change just the
> save and load functions here, as the adjustments are necessary
> independent of the Dom0 aspect. Should num_var_ranges indeed be
> below MTRR_VCNT, there's an information leak here (of hypervisor stack
> data) without pre-initializing hw_mtrr. Here's the hunk from my patch, in
> case you care to re-use parts of it:
> 
> @@ -676,22 +676,22 @@ int hvm_set_mem_pinned_cacheattr(struct
>  
>  static int hvm_save_mtrr_msr(struct domain *d, hvm_domain_context_t *h)
>  {
> -int i;
>  struct vcpu *v;
> -struct hvm_hw_mtrr hw_mtrr;
> -struct mtrr_state *mtrr_state;
> +
>  /* save mtrr */
>  for_each_vcpu(d, v)
>  {
> -mtrr_state = >arch.hvm_vcpu.mtrr;
> +const struct mtrr_state *mtrr_state = >arch.hvm_vcpu.mtrr;
> +struct hvm_hw_mtrr hw_mtrr = {
> +.msr_mtrr_def_type = mtrr_state->def_type |
> + (mtrr_state->enabled << 10),
> +.msr_mtrr_cap  = mtrr_state->mtrr_cap,
> +};
> +unsigned int i;
>  
>  hvm_get_guest_pat(v, _mtrr.msr_pat_cr);
>  
> -hw_mtrr.msr_mtrr_def_type = mtrr_state->def_type
> -| (mtrr_state->enabled << 10);
> -hw_mtrr.msr_mtrr_cap = mtrr_state->mtrr_cap;
> -
> -for ( i = 0; i < MTRR_VCNT; i++ )
> +for ( i = 0; i < (uint8_t)hw_mtrr.msr_mtrr_cap; i++ )
>  {
>  /* save physbase */
>  hw_mtrr.msr_mtrr_var[i*2] =
> 
> (I didn't send it out yet as I'm generally of the opinion that prior to
> branching focus should be on the code to be released rather than
> the next following version.)

Would you be OK if I integrate this as a pre-patch to this one in my
series?

Thanks, Roger.

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

Re: [Xen-devel] [PATCH v6] scripts/add_maintainers.pl: New script

[Ian Jackson]

George Dunlap writes ("Re: [PATCH v6] scripts/add_maintainers.pl: New script"):
> > Changes since v5:
> > - Add mention of --get-maintainers, and its best use case, to --help
> >   output.  (Move $get_maintainer up so that it can be used here.)
> 
> Do you actually want to check this massive changelog into the git repo?

No.  Lars used a nonstandard form for the `changes in vN' information
and I decided not to reformat it.  I will strip it out when I commit
to staging.

Ian.

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

Re: [Xen-devel] [PATCH 10/10] x86/spec_ctrl: Elide MSR_SPEC_CTRL handling in idle context when possible

[Wei Liu]

On Wed, May 16, 2018 at 12:27:29PM +0100, Andrew Cooper wrote:
> On 14/05/18 16:48, Wei Liu wrote:
> > On Fri, May 11, 2018 at 11:38:14AM +0100, Andrew Cooper wrote:
> >> If Xen is virtualising MSR_SPEC_CTRL handling for guests, but using 0 as 
> >> its
> >> own MSR_SPEC_CTRL value, spec_ctrl_{enter,exit}_idle() need not write to 
> >> the
> >> MSR.
> >>
> >> Requested-by: Jan Beulich 
> >> Signed-off-by: Andrew Cooper 
> >> ---
> >> CC: Jan Beulich 
> >> CC: Wei Liu 
> >> CC: Roger Pau Monné 
> >> CC: Juergen Gross 
> >> ---
> >>  xen/arch/x86/spec_ctrl.c  | 4 
> >>  xen/include/asm-x86/cpufeatures.h | 1 +
> >>  xen/include/asm-x86/spec_ctrl.h   | 8 ++--
> >>  3 files changed, 7 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
> >> index a2328bd..f4a3165 100644
> >> --- a/xen/arch/x86/spec_ctrl.c
> >> +++ b/xen/arch/x86/spec_ctrl.c
> >> @@ -526,6 +526,10 @@ void __init init_speculation_mitigations(void)
> >>  /* (Re)init BSP state now that default_spec_ctrl_flags has been 
> >> calculated. */
> >>  init_shadow_spec_ctrl_state();
> >>  
> >> +/* If Xen is using any MSR_SPEC_CTRL settings, adjust the idle path. 
> >> */
> >> +if ( default_xen_spec_ctrl )
> >> +setup_force_cpu_cap(X86_FEATURE_SC_MSR_IDLE);
> >> +
> >>  xpti_init_default(false);
> >>  if ( opt_xpti == 0 )
> >>  setup_force_cpu_cap(X86_FEATURE_NO_XPTI);
> >> diff --git a/xen/include/asm-x86/cpufeatures.h 
> >> b/xen/include/asm-x86/cpufeatures.h
> >> index 9d5d81e..b90aa2d 100644
> >> --- a/xen/include/asm-x86/cpufeatures.h
> >> +++ b/xen/include/asm-x86/cpufeatures.h
> >> @@ -31,3 +31,4 @@ XEN_CPUFEATURE(SC_MSR_HVM,  (FSCAPINTS+0)*32+17) /* 
> >> MSR_SPEC_CTRL used by Xe
> >>  XEN_CPUFEATURE(SC_RSB_PV,   (FSCAPINTS+0)*32+18) /* RSB overwrite 
> >> needed for PV */
> >>  XEN_CPUFEATURE(SC_RSB_HVM,  (FSCAPINTS+0)*32+19) /* RSB overwrite 
> >> needed for HVM */
> >>  XEN_CPUFEATURE(NO_XPTI, (FSCAPINTS+0)*32+20) /* XPTI mitigation 
> >> not in use */
> >> +XEN_CPUFEATURE(SC_MSR_IDLE, (FSCAPINTS+0)*32+21) /* (SC_MSR_PV || 
> >> SC_MSR_HVM) && default_xen_spec_ctrl */
> > I don't follow: the code above only depends on default_xen_spec_ctrl but
> > the comment says SC_MSR_PV and SC_MSR_HVM are also taken into
> > consideration.
> 
> default_xen_spec_ctrl is only ever nonzero when SC_MSR_PV or SC_MSR_HVM
> is set.  In principle, I could assert that one of the two is set.
> 
> I have phrased it like this because it is going to become rather more
> complicated (than != 0) when adding IBRS_ATT mode.

OK, thanks for explaining.

Wei.

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

Re: [Xen-devel] [PATCH 10/10] x86/spec_ctrl: Elide MSR_SPEC_CTRL handling in idle context when possible

[Andrew Cooper]

On 14/05/18 16:48, Wei Liu wrote:
> On Fri, May 11, 2018 at 11:38:14AM +0100, Andrew Cooper wrote:
>> If Xen is virtualising MSR_SPEC_CTRL handling for guests, but using 0 as its
>> own MSR_SPEC_CTRL value, spec_ctrl_{enter,exit}_idle() need not write to the
>> MSR.
>>
>> Requested-by: Jan Beulich 
>> Signed-off-by: Andrew Cooper 
>> ---
>> CC: Jan Beulich 
>> CC: Wei Liu 
>> CC: Roger Pau Monné 
>> CC: Juergen Gross 
>> ---
>>  xen/arch/x86/spec_ctrl.c  | 4 
>>  xen/include/asm-x86/cpufeatures.h | 1 +
>>  xen/include/asm-x86/spec_ctrl.h   | 8 ++--
>>  3 files changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
>> index a2328bd..f4a3165 100644
>> --- a/xen/arch/x86/spec_ctrl.c
>> +++ b/xen/arch/x86/spec_ctrl.c
>> @@ -526,6 +526,10 @@ void __init init_speculation_mitigations(void)
>>  /* (Re)init BSP state now that default_spec_ctrl_flags has been 
>> calculated. */
>>  init_shadow_spec_ctrl_state();
>>  
>> +/* If Xen is using any MSR_SPEC_CTRL settings, adjust the idle path. */
>> +if ( default_xen_spec_ctrl )
>> +setup_force_cpu_cap(X86_FEATURE_SC_MSR_IDLE);
>> +
>>  xpti_init_default(false);
>>  if ( opt_xpti == 0 )
>>  setup_force_cpu_cap(X86_FEATURE_NO_XPTI);
>> diff --git a/xen/include/asm-x86/cpufeatures.h 
>> b/xen/include/asm-x86/cpufeatures.h
>> index 9d5d81e..b90aa2d 100644
>> --- a/xen/include/asm-x86/cpufeatures.h
>> +++ b/xen/include/asm-x86/cpufeatures.h
>> @@ -31,3 +31,4 @@ XEN_CPUFEATURE(SC_MSR_HVM,  (FSCAPINTS+0)*32+17) /* 
>> MSR_SPEC_CTRL used by Xe
>>  XEN_CPUFEATURE(SC_RSB_PV,   (FSCAPINTS+0)*32+18) /* RSB overwrite 
>> needed for PV */
>>  XEN_CPUFEATURE(SC_RSB_HVM,  (FSCAPINTS+0)*32+19) /* RSB overwrite 
>> needed for HVM */
>>  XEN_CPUFEATURE(NO_XPTI, (FSCAPINTS+0)*32+20) /* XPTI mitigation not 
>> in use */
>> +XEN_CPUFEATURE(SC_MSR_IDLE, (FSCAPINTS+0)*32+21) /* (SC_MSR_PV || 
>> SC_MSR_HVM) && default_xen_spec_ctrl */
> I don't follow: the code above only depends on default_xen_spec_ctrl but
> the comment says SC_MSR_PV and SC_MSR_HVM are also taken into
> consideration.

default_xen_spec_ctrl is only ever nonzero when SC_MSR_PV or SC_MSR_HVM
is set.  In principle, I could assert that one of the two is set.

I have phrased it like this because it is going to become rather more
complicated (than != 0) when adding IBRS_ATT mode.

~Andrew

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

Re: [Xen-devel] [PATCH v6] scripts/add_maintainers.pl: New script

[George Dunlap]

On 05/15/2018 06:23 PM, Ian Jackson wrote:
> From: Lars Kurth 
> 
> This provides a much better workflow when using git format-patch and
> git send-email, with get_maintainer.pl.
> 
> The tool covers step 2 of the following workflow
> 
>   Step 1: git format-patch ... -o  ...
>   Step 2: ./scripts/add_maintainers.pl -d 
>   This overwrites  *.patch files in 
>   Step 3: git send-email -to xen-devel@lists.xenproject.org 
> /*.patchxm
> 
> I manually tested all options and the most common combinations
> on Mac.
> 
> Changes since v1:
> - Added RAB (indicated by Juergen on IRC that this is OK)
> - Remove trailing whitespaces
> - Renamed --prefix to --reroll-count
> - Cleaned up short options -v, ... to be in line with git
> - Added --tags|-t option to add AB, RAB and RB emails to CC list
> - Added --insert|-i mode to allow for people adding CCs to commit message
>   instead of the e-mail header (the header is the default)
> - Moved common code into functions
> - Added logic, such that the tool only insert's To: and Cc: statements
>   which were not there before, allowing for running the tool multiple times
>   on the same 
> 
> Changes since v2:
> - Deleted --version and related infrastructure
> - Added subroutine prototypes
> - Removed AT and @lists declaration and used \@ in literals
> - Changed usage message and options based on feedback
> - Improved error handling
> - Removed occurances of index() and replaced with regex
> - Removed non-perl idioms
> - Moved uniq statements to normalize and added info on what normalize does
> - Read L: tags from MAINTAINERS file instead of using heuristic
> - Fixed issues related to metacharacters in getmaintainers()
> - Allow multiple -a | --arg values (because of this renamed --args)
> - Identify tags via regex
> - CC's from tags are only inserted in the mail header, never the body
> - That is unless the new option --tagscc is used
> - Added policy processing which includes reworking insert()
> - Replaced -i|--insert with -p|--inspatch and -c|--inscover now using policies
> - Added new policies to cover for all user requests
> - Rewrote help message to center around usage of policies
> - Reordered some code (e.g. help string first to make code more easily 
> readable)
> 
> Changes since v3:
> - Made help message clearer
> - Replaced PROCESSING POLICY with LOCATION
> - Renamed --inspatch (top|ccbody|cc---|none) | -p (top|ccbody|cc---|none)
>   to --patchcc (header|commit|comment|none) | -p (header|commit|comment|none)
> - Renamed --inscover (top|ccend|none) | -c (top|ccend|none)
>   to --covercc (header|end|none) | -c (header|end|none)
> - Renamed variables and functions in the code to match the options
> - Changed $patch_prefix processing
> - Changed search expression for identifying cover letters
> - Renamed $readmailinglists to $getmailinglists_done
> - Use array form of open
> - More file error handling (using IO::Handle)
> - Fixed buggy AND in if statement
> - Removed check whether getmaintainers exists for future proofing
> - Add logic to work out --reroll-count
> 
> Changes since v4:
> - Strip some trailing whitespace from the code
> - writefile() now uses the .tmp-and-rename pattern to avoid data loss
> - Provide --get-maintainers= option to specify replacement for
>   get_maintainers.pl.  This is useful for Ian's usecase, since it
>   allows --get-maintainers=true, to avoid adding any MAINTAINERS-based
>   info anywhere while still adding other CCs (eg from -t) everywhere.
> - Refactor normalize() somewhat so that it uses only %seen, and
>   does not any longer modify its argument arrays.
> - De-dupe case-insensitively (by making normalize use lc).
> 
> Changes since v5:
> - Add mention of --get-maintainers, and its best use case, to --help
>   output.  (Move $get_maintainer up so that it can be used here.)

Do you actually want to check this massive changelog into the git repo?

 -George

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

Re: [Xen-devel] [PATCH 07/10] x86/spec_ctrl: Explicitly set Xen's default MSR_SPEC_CTRL value

[Wei Liu]

On Wed, May 16, 2018 at 12:08:02PM +0100, Andrew Cooper wrote:
> On 14/05/18 16:52, Jan Beulich wrote:
>  On 14.05.18 at 17:39,  wrote:
> >> On Fri, May 11, 2018 at 11:38:11AM +0100, Andrew Cooper wrote:
> >>> @@ -417,6 +419,32 @@ void __init init_speculation_mitigations(void)
> >>>  setup_clear_cpu_cap(X86_FEATURE_NO_XPTI);
> >>>  
> >>>  print_details(thunk, caps);
> >>> +
> >>> +/*
> >>> + * If MSR_SPEC_CTRL is available, apply Xen's default setting and 
> >>> discard
> >>> + * any firmware settings.  For performance reasons on native 
> >>> hardware, we
> >>> + * delay applying non-zero settings until after dom0 has been 
> >>> constructed.
> >>> + */
> >>> +if ( boot_cpu_has(X86_FEATURE_IBRSB) )
> >>> +{
> >>> +bsp_delay_spec_ctrl = !cpu_has_hypervisor && 
> >>> default_xen_spec_ctrl;
> >>> +
> >> Why is cpu_has_hypervisor needed here?  This should help nested case as
> >> well. And it wouldn't make the setup less secure, right?
> > Ah, yes, Andrew, this should indeed be explained in at least one of comment
> > or commit message.
> 
> I've adjusted this comment to read:
> 
> /*
>  * If MSR_SPEC_CTRL is available, apply Xen's default setting and discard
>  * any firmware settings.  For performance reasons, when safe to do so, we
>  * delay applying non-zero settings until after dom0 has been constructed.
>  *
>  * "when safe to do so" is based on whether we are virtualised.  A native
>  * boot won't have any other code running in a position to mount an
>  * attack.
>  */
> 
> and added the same second paragraph to the commit message.

LGTM. Thanks!

Wei.

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

Re: [Xen-devel] [PATCH 07/10] x86/spec_ctrl: Explicitly set Xen's default MSR_SPEC_CTRL value

[Andrew Cooper]

On 14/05/18 16:52, Jan Beulich wrote:
 On 14.05.18 at 17:39,  wrote:
>> On Fri, May 11, 2018 at 11:38:11AM +0100, Andrew Cooper wrote:
>>> @@ -417,6 +419,32 @@ void __init init_speculation_mitigations(void)
>>>  setup_clear_cpu_cap(X86_FEATURE_NO_XPTI);
>>>  
>>>  print_details(thunk, caps);
>>> +
>>> +/*
>>> + * If MSR_SPEC_CTRL is available, apply Xen's default setting and 
>>> discard
>>> + * any firmware settings.  For performance reasons on native hardware, 
>>> we
>>> + * delay applying non-zero settings until after dom0 has been 
>>> constructed.
>>> + */
>>> +if ( boot_cpu_has(X86_FEATURE_IBRSB) )
>>> +{
>>> +bsp_delay_spec_ctrl = !cpu_has_hypervisor && default_xen_spec_ctrl;
>>> +
>> Why is cpu_has_hypervisor needed here?  This should help nested case as
>> well. And it wouldn't make the setup less secure, right?
> Ah, yes, Andrew, this should indeed be explained in at least one of comment
> or commit message.

I've adjusted this comment to read:

/*

 * If MSR_SPEC_CTRL is available, apply Xen's default setting and discard

 * any firmware settings.  For performance reasons, when safe to do so, we

 * delay applying non-zero settings until after dom0 has been constructed.

 *

 * "when safe to do so" is based on whether we are virtualised.  A native

 * boot won't have any other code running in a position to mount an

 * attack.

 */


and added the same second paragraph to the commit message.

~Andrew

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

Re: [Xen-devel] [PATCH 06/10] x86/spec_ctrl: Split X86_FEATURE_SC_MSR into PV and HVM variants

[Andrew Cooper]

On 16/05/18 11:49, Jan Beulich wrote:
 On 16.05.18 at 12:28,  wrote:
>> On 16/05/18 07:38, Jan Beulich wrote:
>> On 15.05.18 at 21:52,  wrote:
 On 14/05/18 16:27, Jan Beulich wrote:
 On 11.05.18 at 12:38,  wrote:
>> --- a/xen/arch/x86/spec_ctrl.c
>> +++ b/xen/arch/x86/spec_ctrl.c
>> @@ -128,7 +128,8 @@ static void __init print_details(enum ind_thunk 
>> thunk, 
>> uint64_t caps)
>> thunk == THUNK_RETPOLINE ? "RETPOLINE" :
>> thunk == THUNK_LFENCE? "LFENCE" :
>> thunk == THUNK_JMP   ? "JMP" : "?",
>> -   boot_cpu_has(X86_FEATURE_SC_MSR) ?
>> +   (boot_cpu_has(X86_FEATURE_SC_MSR_PV) ||
>> +boot_cpu_has(X86_FEATURE_SC_MSR_HVM)) ?
>> default_xen_spec_ctrl & SPEC_CTRL_IBRS? " IBRS+" :
>> " IBRS-"  : 
>> "",
>> opt_ibpb  ? " IBPB"   : 
>> "",
>> @@ -367,7 +368,8 @@ void __init init_speculation_mitigations(void)
>>   * need the IBRS entry/exit logic to virtualise IBRS support for
>>   * guests.
>>   */
>> -setup_force_cpu_cap(X86_FEATURE_SC_MSR);
>> +setup_force_cpu_cap(X86_FEATURE_SC_MSR_PV);
>> +setup_force_cpu_cap(X86_FEATURE_SC_MSR_HVM);
> Besides these sort of open coding alternative_io_2() (you'd really want an
> output-less variant here, I agree) these are slightly bending the rules of
> when/how to use multiple alternatives: The above ends up correct only
> because of both replacements being identical.
 Actually, by reordering patch 10 ahead of this patch, we never get to
 needing the ALTERNATIVE_2()'s in the first place, and lose any concerns
 with bending the rules along the series.
>>> Ah yes, indeed. And you would better use alternative_input() there then,
>>> instead of open coding it.
>> The reason this doesn't use alternative_input() at the moment is because
>> of the memory clobber.  (And the lack of a memory clobber is called out
>> as a peculiarity in comment).  The current code looks dangerously
>> inconsistent WRT barriers.
>>
>> As for bending the rules, I now disagree with your assessment.  The
>> alternative_*() wrappers do nothing but make it harder to express the
>> parameters, as perfectly demonstrated by the ASM_OUTPUT2() bodge.
> The "bending the rules" comment was unrelated to alternative_*() vs
> ALTERNATIVE*() use, and instead was solely related to there being a
> dependency here on both pieces of replacement code being identical.
>
>> I don't see their value, and they have a cost of making an asm volatile
>> statement not look and work quite as an asm volatile statement does in
>> all other callsites.
> I don't mind consistency being achieved to other way around (i.e. by
> dropping those wrappers). But I'd prefer if we didn't mix things unless
> there's a compelling reason to do so.

I'll see about doing some cleanup of the overall tree for 4.12.  For
now, its already mixed, and this doesn't make anything worse.

~Andrew

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

[Xen-devel] Unable to compile blktap-3.2.0.xs1086 against kernel 4.16.8

[Rishi]

Dear All,

I'm trying to rebuild the blktap module from xenserver. I'm getting
following compile errors, any help or direction would be appreciated.

make[2]: Entering directory
`/root/rpmbuild/BUILD/blktap-3.2.0.xs1086/drivers'

/bin/sh ../libtool  --tag=CC   --mode=compile gcc -DHAVE_CONFIG_H -I.
-I..  -D_GNU_SOURCE
-I../include  -Wall -Werror -DXC_WANT_COMPAT_EVTCHN_API
-DXC_WANT_COMPAT_GNTTAB_API -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2
-fexceptions -fstack-protector-strong --param=ssp-buffer-size=4
-grecord-gcc-switches   -m64 -mtune=generic -c -o td-req.lo td-req.c

libtool: compile:  gcc -DHAVE_CONFIG_H -I. -I.. -D_GNU_SOURCE -I../include
-Wall -Werror -DXC_WANT_COMPAT_EVTCHN_API -DXC_WANT_COMPAT_GNTTAB_API -O2
-g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
-fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches
-m64 -mtune=generic -c td-req.c  -fPIC -DPIC -o .libs/td-req.o

*td-req.c:* In function '*guest_copy2*':

*td-req.c:359:18:* *error: *'*struct gntdev_grant_copy_segment*' has no
member named '*iov*'

 gcopy_seg->iov.iov_base = tapreq->vma + (i << PAGE_SHIFT) +
(blkif_seg->first_sect << SECTOR_SHIFT);

*  ^*

*td-req.c:360:18:* *error: *'*struct gntdev_grant_copy_segment*' has no
member named '*iov*'

 gcopy_seg->iov.iov_len = (blkif_seg->last_sect -
blkif_seg->first_sect + 1) << SECTOR_SHIFT;

*  ^*

*td-req.c:361:18:* *error: *'*struct gntdev_grant_copy_segment*' has no
member named '*ref*'

 gcopy_seg->ref = blkif_seg->gref;

*  ^*

*td-req.c:362:18:* *error: *'*struct gntdev_grant_copy_segment*' has no
member named '*offset*'

 gcopy_seg->offset = blkif_seg->first_sect << SECTOR_SHIFT;

*  ^*

*td-req.c:365:10:* *error: *'*struct ioctl_gntdev_grant_copy*' has no
member named '*dir*'

 gcopy.dir = blkif_rq_wr(>msg);

*  ^*

*td-req.c:366:10:* *error: *'*struct ioctl_gntdev_grant_copy*' has no
member named '*domid*'

 gcopy.domid = blkif->domid;

*  ^*

make[2]: *** [td-req.lo] Error 1

make[2]: Leaving directory
`/root/rpmbuild/BUILD/blktap-3.2.0.xs1086/drivers'

make[1]: *** [all-recursive] Error 1

make[1]: Leaving directory `/root/rpmbuild/BUILD/blktap-3.2.0.xs1086'

make: *** [all] Error 2


I can share updated structures on Xen side but not sure how to modify above
code to let it adhere to latest xen.


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

Re: [Xen-devel] [PATCH v2 5/6] libxc/pvh: set default MTRR type to write-back

[Wei Liu]

On Tue, May 15, 2018 at 03:36:16PM +0100, Roger Pau Monne wrote:
> And enable MTRR. This allows to provide a sane initial MTRR state for
> PVH DomUs. This will have to be expanded when pci-passthrough support
> is added to PVH guests, so that MMIO regions of devices are set as
> UC.
> 
> Note that initial MTRR setup is done by hvmloader for HVM guests,
> that's not used by PVH guests.
> 
> Signed-off-by: Roger Pau Monné 

Acked-by: Wei Liu 

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

[Xen-devel] [xen-unstable-coverity test] 122867: all pass - PUSHED

[osstest service owner]

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

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 xen  b953322c5772dbc537421f9e2f97026a1c2fcb2e
baseline version:
 xen  858dbaaeda33b05c1ac80aea0ba9a03924e09005

Last test of basis   122736  2018-05-13 09:18:19 Z3 days
Testing same since   122867  2018-05-16 09:18:19 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Ian Jackson 
  Konrad Rzeszutek Wilk 
  Lars Kurth 
  Oleksandr Andrushchenko 
  Paul Durrant 

jobs:
 coverity-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/xen.git
   858dbaaeda..b953322c57  b953322c5772dbc537421f9e2f97026a1c2fcb2e -> 
coverity-tested/smoke

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

Re: [Xen-devel] [PATCH 06/10] x86/spec_ctrl: Split X86_FEATURE_SC_MSR into PV and HVM variants

[Andrew Cooper]

On 16/05/18 07:38, Jan Beulich wrote:
 On 15.05.18 at 21:52,  wrote:
>> On 14/05/18 16:27, Jan Beulich wrote:
>> On 11.05.18 at 12:38,  wrote:
 --- a/xen/arch/x86/spec_ctrl.c
 +++ b/xen/arch/x86/spec_ctrl.c
 @@ -128,7 +128,8 @@ static void __init print_details(enum ind_thunk thunk, 
 uint64_t caps)
 thunk == THUNK_RETPOLINE ? "RETPOLINE" :
 thunk == THUNK_LFENCE? "LFENCE" :
 thunk == THUNK_JMP   ? "JMP" : "?",
 -   boot_cpu_has(X86_FEATURE_SC_MSR) ?
 +   (boot_cpu_has(X86_FEATURE_SC_MSR_PV) ||
 +boot_cpu_has(X86_FEATURE_SC_MSR_HVM)) ?
 default_xen_spec_ctrl & SPEC_CTRL_IBRS? " IBRS+" :
 " IBRS-"  : "",
 opt_ibpb  ? " IBPB"   : "",
 @@ -367,7 +368,8 @@ void __init init_speculation_mitigations(void)
   * need the IBRS entry/exit logic to virtualise IBRS support for
   * guests.
   */
 -setup_force_cpu_cap(X86_FEATURE_SC_MSR);
 +setup_force_cpu_cap(X86_FEATURE_SC_MSR_PV);
 +setup_force_cpu_cap(X86_FEATURE_SC_MSR_HVM);
>>> Besides these sort of open coding alternative_io_2() (you'd really want an
>>> output-less variant here, I agree) these are slightly bending the rules of
>>> when/how to use multiple alternatives: The above ends up correct only
>>> because of both replacements being identical.
>> Actually, by reordering patch 10 ahead of this patch, we never get to
>> needing the ALTERNATIVE_2()'s in the first place, and lose any concerns
>> with bending the rules along the series.
> Ah yes, indeed. And you would better use alternative_input() there then,
> instead of open coding it.

The reason this doesn't use alternative_input() at the moment is because
of the memory clobber.  (And the lack of a memory clobber is called out
as a peculiarity in comment).  The current code looks dangerously
inconsistent WRT barriers.

As for bending the rules, I now disagree with your assessment.  The
alternative_*() wrappers do nothing but make it harder to express the
parameters, as perfectly demonstrated by the ASM_OUTPUT2() bodge.

I don't see their value, and they have a cost of making an asm volatile
statement not look and work quite as an asm volatile statement does in
all other callsites.

~Andrew

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

Re: [Xen-devel] [PATCH] xen-hvm: stop faking I/O to access PCI config space

[Roger Pau Monné]

On Thu, May 03, 2018 at 12:18:40PM +0100, Paul Durrant wrote:
> This patch removes the current hackery where IOREQ_TYPE_PCI_CONFIG
> reqyests are handled by faking PIO to 0xcf8 and 0xcfc and replaces it
> with direct calls to pci_host_config_read/write_common().
> Doing so necessitates mapping BDFs to PCIDevices but maintaining a simple
> QLIST in xen_device_realize/unrealize() will suffice.
> 
> NOTE: whilst config space accesses are currently limited to
>   PCI_CONFIG_SPACE_SIZE, this patch paves the way to increasing the
>   limit to PCIE_CONFIG_SPACE_SIZE when Xen gains the ability to
>   emulate MCFG table accesses.

Thanks for doing this. I'm not a QEMU maintainer but:

Reviewed-by: Roger Pau Monné 

> Signed-off-by: Paul Durrant 
> --
> Cc: Stefano Stabellini 
> Cc: Anthony Perard 
> Cc: "Michael S. Tsirkin" 
> Cc: Marcel Apfelbaum 
> Cc: Paolo Bonzini 
> Cc: Richard Henderson 
> Cc: Eduardo Habkost 
> ---
>  hw/i386/xen/trace-events |   2 +
>  hw/i386/xen/xen-hvm.c| 101 
> +--
>  2 files changed, 83 insertions(+), 20 deletions(-)
> 
> diff --git a/hw/i386/xen/trace-events b/hw/i386/xen/trace-events
> index 8dab7bc..f576f1b 100644
> --- a/hw/i386/xen/trace-events
> +++ b/hw/i386/xen/trace-events
> @@ -15,6 +15,8 @@ cpu_ioreq_pio(void *req, uint32_t dir, uint32_t df, 
> uint32_t data_is_ptr, uint64
>  cpu_ioreq_pio_read_reg(void *req, uint64_t data, uint64_t addr, uint32_t 
> size) "I/O=%p pio read reg data=0x%"PRIx64" port=0x%"PRIx64" size=%d"
>  cpu_ioreq_pio_write_reg(void *req, uint64_t data, uint64_t addr, uint32_t 
> size) "I/O=%p pio write reg data=0x%"PRIx64" port=0x%"PRIx64" size=%d"
>  cpu_ioreq_move(void *req, uint32_t dir, uint32_t df, uint32_t data_is_ptr, 
> uint64_t addr, uint64_t data, uint32_t count, uint32_t size) "I/O=%p copy 
> dir=%d df=%d ptr=%d port=0x%"PRIx64" data=0x%"PRIx64" count=%d size=%d"
> +cpu_ioreq_config_read(void *req, uint32_t sbdf, uint32_t reg, uint32_t size, 
> uint32_t data) "I/O=%p sbdf=0x%x reg=%u size=%u data=0x%x"
> +cpu_ioreq_config_write(void *req, uint32_t sbdf, uint32_t reg, uint32_t 
> size, uint32_t data) "I/O=%p sbdf=0x%x reg=%u size=%u data=0x%x"
>  
>  # xen-mapcache.c
>  xen_map_cache(uint64_t phys_addr) "want 0x%"PRIx64
> diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
> index caa563b..c139d29 100644
> --- a/hw/i386/xen/xen-hvm.c
> +++ b/hw/i386/xen/xen-hvm.c
> @@ -12,6 +12,7 @@
>  
>  #include "cpu.h"
>  #include "hw/pci/pci.h"
> +#include "hw/pci/pci_host.h"
>  #include "hw/i386/pc.h"
>  #include "hw/i386/apic-msidef.h"
>  #include "hw/xen/xen_common.h"
> @@ -86,6 +87,12 @@ typedef struct XenPhysmap {
>  QLIST_ENTRY(XenPhysmap) list;
>  } XenPhysmap;
>  
> +typedef struct XenPciDevice {
> +PCIDevice *pci_dev;
> +uint32_t sbdf;
> +QLIST_ENTRY(XenPciDevice) entry;
> +} XenPciDevice;
> +
>  typedef struct XenIOState {
>  ioservid_t ioservid;
>  shared_iopage_t *shared_page;
> @@ -105,6 +112,7 @@ typedef struct XenIOState {
>  struct xs_handle *xenstore;
>  MemoryListener memory_listener;
>  MemoryListener io_listener;
> +QLIST_HEAD(, XenPciDevice) dev_list;
>  DeviceListener device_listener;
>  QLIST_HEAD(, XenPhysmap) physmap;
>  hwaddr free_phys_offset;
> @@ -569,6 +577,12 @@ static void xen_device_realize(DeviceListener *listener,
>  
>  if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
>  PCIDevice *pci_dev = PCI_DEVICE(dev);
> +XenPciDevice *xendev = g_new(XenPciDevice, 1);
> +
> +xendev->pci_dev = pci_dev;
> +xendev->sbdf = PCI_BUILD_BDF(pci_dev_bus_num(pci_dev),
> + pci_dev->devfn);
> +QLIST_INSERT_HEAD(>dev_list, xendev, entry);
>  
>  xen_map_pcidev(xen_domid, state->ioservid, pci_dev);
>  }
> @@ -581,8 +595,17 @@ static void xen_device_unrealize(DeviceListener 
> *listener,
>  
>  if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
>  PCIDevice *pci_dev = PCI_DEVICE(dev);
> +XenPciDevice *xendev, *next;
>  
>  xen_unmap_pcidev(xen_domid, state->ioservid, pci_dev);
> +
> +QLIST_FOREACH_SAFE(xendev, >dev_list, entry, next) {
> +if (xendev->pci_dev == pci_dev) {
> +QLIST_REMOVE(xendev, entry);
> +g_free(xendev);
> +break;
> +}
> +}
>  }
>  }
>  
> @@ -903,6 +926,61 @@ static void cpu_ioreq_move(ioreq_t *req)
>  }
>  }
>  
> +static void cpu_ioreq_config(XenIOState *state, ioreq_t *req)
> +{
> +uint32_t sbdf = req->addr >> 32;
> +uint32_t reg = req->addr;
> +XenPciDevice *xendev;
> +
> +if (req->size > sizeof(uint32_t)) {
> +hw_error("PCI config access: bad size (%u)", req->size);
> 

Re: [Xen-devel] [RFC][PATCH] KVM: APPLES can improve the performance of applications and virtualized systems by up to 49%

[David Hildenbrand]

On 12.05.2018 10:27, Weiwei Jia wrote:
> Dear all,
> 
> Recently, we made a few improvements on effectively utilizing Pause
> Loop Exiting (PLE) support for higher throughput on virtualized
> systems. Basically, it solves two problems: 1) how to adjust
> PLE_Window; 2) how to select virtual CPUs to schedule on VM_EXITs
> caused by PLE. Our tests with standard benchmarks show that the
> approach can improve performance by up to 49%. The approach shows
> promising performance and is easy to implement. We think that it would
> be wonderful if Linux/KVM and XEN can consider the approach.
> 
> We already have a prototype implementation based on KVM (Linux Kernel
> 3.19.8). Our patch for Linux Kernel 3.19.8 and the paper describing
> our idea are available in Github repository [1][2][3]. We are pleased
> to revise our patch in order to merge it into Linux/KVM and XEN. We
> hope that you can test and adopt our approach/techniques. We are
> pleased to get some comments/suggestions on the approach and on how
> the idea can be adopted/tested by Linux/KVM and XEN. Thank you.
> 

Hi,

Please port the patch to latest upstream and send it as a proper patch
to this mailing list. (otherwise you won't get feedback on it WHP)

If you want some initial comment if this makes sense at all in the
context of KVM and can be implemented, you can send the patch itself as
RFC (based on an older kernel version).

Thanks!

> [1] APPLES paper: https://github.com/sysmen/apples/tree/master/paper
> [2] APPLES patch:
> https://github.com/sysmen/apples/blob/master/patches/3.19.8-APPLES.patch
> [3] APPLES patch README:
> https://github.com/sysmen/apples/blob/master/patches/README.txt
> 
> Best Regards,
> Sysmen Research Group
> 


-- 

Thanks,

David / dhildenb

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

[Xen-devel] Clarification Meltdown for vulnerability between 64-bit PV guests

[John Wang]

Hi:

64 bit PV guest can attack hypervisor  by SP3, whether it still can 
attack others 64 bit PV guest by SP3 ? If yes, whether the xpti enable 
on hypervisor can prevent  vulnerability?  if no, what operations need 
to be done on 64 PV guest or hypervisor?



--
Thanks
APACII QA
John(XiaoGen Wang)

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

Re: [Xen-devel] [PATCH v2 09/11] docs: Fix some broken references

[Charles Keepax]

On Wed, May 09, 2018 at 10:18:52AM -0300, Mauro Carvalho Chehab wrote:
> As we move stuff around, some doc references are broken. Fix some of
> them via this script:
>   ./scripts/documentation-file-ref-check --fix-rst
> 
> Manually checked if the produced result is valid, removing a few
> false-positives.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/input/touchscreen/wm97xx-core.c   |  2 +-

Acked-by: Charles Keepax 

Thanks,
Charles

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

[Xen-devel] [xen-4.8-testing test] 122771: FAIL

[osstest service owner]

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

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-arm64-arm64-xl-credit2  broken  in 122704
 test-arm64-arm64-xl-xsm  broken  in 122704

Tests which are failing intermittently (not blocking):
 test-arm64-arm64-xl-credit2  4 host-install(4) broken in 122704 pass in 122771
 test-arm64-arm64-xl-xsm  4 host-install(4) broken in 122704 pass in 122771
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 16 guest-localmigrate/x10 
fail in 122704 pass in 122771
 test-amd64-amd64-xl-qemuu-ws16-amd64 16 guest-localmigrate/x10 fail pass in 
122704

Tests which did not succeed, but are not blocking:
 test-xtf-amd64-amd64-1 50 xtf/test-hvm64-lbr-tsx-vmentry fail in 122704 like 
122508
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop  fail in 122704 like 122508
 test-xtf-amd64-amd64-4  50 xtf/test-hvm64-lbr-tsx-vmentry fail like 122466
 test-xtf-amd64-amd64-5  50 xtf/test-hvm64-lbr-tsx-vmentry fail like 122508
 test-xtf-amd64-amd64-2  50 xtf/test-hvm64-lbr-tsx-vmentry fail like 122508
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 122508
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 122508
 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeatfail  like 122508
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 122508
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 122508
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 122508
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 122508
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail like 122508
 build-i386-prev   7 xen-build/dist-test  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-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-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-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 build-amd64-prev  7 xen-build/dist-test  fail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 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-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  13 migrate-support-check

[Xen-devel] backporting considerations (Re: [PATCH v9 0/9] xen/x86: various XPTI speedups)

[Jan Beulich]

>>> On 26.04.18 at 13:33,  wrote:
> Juergen Gross (9):
>   x86/xpti: avoid copying L4 page table contents when possible
>   xen/x86: add a function for modifying cr3
>   xen/x86: support per-domain flag for xpti
>   xen/x86: use invpcid for flushing the TLB
>   xen/x86: disable global pages for domains with XPTI active
>   xen/x86: use flag byte for decision whether xen_cr3 is valid
>   xen/x86: convert pv_guest_cr4_to_real_cr4() to a function
>   xen/x86: add some cr3 helpers
>   xen/x86: use PCID feature

This being a performance improvement rather than a plain bug fix series,
I'm not entirely certain about backporting here. My current thinking is to
put this into 4.10 (Jürgen was kind enough to do the backporting work
already), but not into any older trees. Otoh at SUSE we already have
this in our 4.9-based branch as well, and if other consumers of that or
the 4.8 branch would mostly agree it should go there, I could certainly
be convinced. It is imo out of question of putting it into 4.7 or older.

Jan


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

Re: [Xen-devel] [PATCH] xen-hvm: stop faking I/O to access PCI config space

[Paul Durrant]

Anthony, Stefano,

  Ping?

  Paul

> -Original Message-
> From: Paul Durrant [mailto:paul.durr...@citrix.com]
> Sent: 03 May 2018 12:19
> To: xen-devel@lists.xenproject.org; qemu-de...@nongnu.org
> Cc: Paul Durrant ; Stefano Stabellini
> ; Anthony Perard ;
> Michael S. Tsirkin ; Marcel Apfelbaum
> ; Paolo Bonzini ; Richard
> Henderson ; Eduardo Habkost 
> Subject: [PATCH] xen-hvm: stop faking I/O to access PCI config space
> 
> This patch removes the current hackery where IOREQ_TYPE_PCI_CONFIG
> reqyests are handled by faking PIO to 0xcf8 and 0xcfc and replaces it
> with direct calls to pci_host_config_read/write_common().
> Doing so necessitates mapping BDFs to PCIDevices but maintaining a simple
> QLIST in xen_device_realize/unrealize() will suffice.
> 
> NOTE: whilst config space accesses are currently limited to
>   PCI_CONFIG_SPACE_SIZE, this patch paves the way to increasing the
>   limit to PCIE_CONFIG_SPACE_SIZE when Xen gains the ability to
>   emulate MCFG table accesses.
> 
> Signed-off-by: Paul Durrant 
> --
> Cc: Stefano Stabellini 
> Cc: Anthony Perard 
> Cc: "Michael S. Tsirkin" 
> Cc: Marcel Apfelbaum 
> Cc: Paolo Bonzini 
> Cc: Richard Henderson 
> Cc: Eduardo Habkost 
> ---
>  hw/i386/xen/trace-events |   2 +
>  hw/i386/xen/xen-hvm.c| 101
> +--
>  2 files changed, 83 insertions(+), 20 deletions(-)
> 
> diff --git a/hw/i386/xen/trace-events b/hw/i386/xen/trace-events
> index 8dab7bc..f576f1b 100644
> --- a/hw/i386/xen/trace-events
> +++ b/hw/i386/xen/trace-events
> @@ -15,6 +15,8 @@ cpu_ioreq_pio(void *req, uint32_t dir, uint32_t df,
> uint32_t data_is_ptr, uint64
>  cpu_ioreq_pio_read_reg(void *req, uint64_t data, uint64_t addr, uint32_t
> size) "I/O=%p pio read reg data=0x%"PRIx64" port=0x%"PRIx64" size=%d"
>  cpu_ioreq_pio_write_reg(void *req, uint64_t data, uint64_t addr, uint32_t
> size) "I/O=%p pio write reg data=0x%"PRIx64" port=0x%"PRIx64" size=%d"
>  cpu_ioreq_move(void *req, uint32_t dir, uint32_t df, uint32_t data_is_ptr,
> uint64_t addr, uint64_t data, uint32_t count, uint32_t size) "I/O=%p copy
> dir=%d df=%d ptr=%d port=0x%"PRIx64" data=0x%"PRIx64" count=%d
> size=%d"
> +cpu_ioreq_config_read(void *req, uint32_t sbdf, uint32_t reg, uint32_t
> size, uint32_t data) "I/O=%p sbdf=0x%x reg=%u size=%u data=0x%x"
> +cpu_ioreq_config_write(void *req, uint32_t sbdf, uint32_t reg, uint32_t
> size, uint32_t data) "I/O=%p sbdf=0x%x reg=%u size=%u data=0x%x"
> 
>  # xen-mapcache.c
>  xen_map_cache(uint64_t phys_addr) "want 0x%"PRIx64
> diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
> index caa563b..c139d29 100644
> --- a/hw/i386/xen/xen-hvm.c
> +++ b/hw/i386/xen/xen-hvm.c
> @@ -12,6 +12,7 @@
> 
>  #include "cpu.h"
>  #include "hw/pci/pci.h"
> +#include "hw/pci/pci_host.h"
>  #include "hw/i386/pc.h"
>  #include "hw/i386/apic-msidef.h"
>  #include "hw/xen/xen_common.h"
> @@ -86,6 +87,12 @@ typedef struct XenPhysmap {
>  QLIST_ENTRY(XenPhysmap) list;
>  } XenPhysmap;
> 
> +typedef struct XenPciDevice {
> +PCIDevice *pci_dev;
> +uint32_t sbdf;
> +QLIST_ENTRY(XenPciDevice) entry;
> +} XenPciDevice;
> +
>  typedef struct XenIOState {
>  ioservid_t ioservid;
>  shared_iopage_t *shared_page;
> @@ -105,6 +112,7 @@ typedef struct XenIOState {
>  struct xs_handle *xenstore;
>  MemoryListener memory_listener;
>  MemoryListener io_listener;
> +QLIST_HEAD(, XenPciDevice) dev_list;
>  DeviceListener device_listener;
>  QLIST_HEAD(, XenPhysmap) physmap;
>  hwaddr free_phys_offset;
> @@ -569,6 +577,12 @@ static void xen_device_realize(DeviceListener
> *listener,
> 
>  if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
>  PCIDevice *pci_dev = PCI_DEVICE(dev);
> +XenPciDevice *xendev = g_new(XenPciDevice, 1);
> +
> +xendev->pci_dev = pci_dev;
> +xendev->sbdf = PCI_BUILD_BDF(pci_dev_bus_num(pci_dev),
> + pci_dev->devfn);
> +QLIST_INSERT_HEAD(>dev_list, xendev, entry);
> 
>  xen_map_pcidev(xen_domid, state->ioservid, pci_dev);
>  }
> @@ -581,8 +595,17 @@ static void xen_device_unrealize(DeviceListener
> *listener,
> 
>  if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
>  PCIDevice *pci_dev = PCI_DEVICE(dev);
> +XenPciDevice *xendev, *next;
> 
>  xen_unmap_pcidev(xen_domid, state->ioservid, pci_dev);
> +
> +QLIST_FOREACH_SAFE(xendev, >dev_list, entry, next) {
> +if (xendev->pci_dev == pci_dev) {
> +QLIST_REMOVE(xendev, entry);
> +g_free(xendev);
> +   

Re: [Xen-devel] [PATCH v2 4/6] hvm/mtrr: copy hardware state for Dom0

[Jan Beulich]

>>> On 15.05.18 at 16:36,  wrote:
> --- a/xen/arch/x86/hvm/mtrr.c
> +++ b/xen/arch/x86/hvm/mtrr.c
> @@ -185,6 +185,30 @@ int hvm_vcpu_cacheattr_init(struct vcpu *v)
>  ((uint64_t)PAT_TYPE_UC_MINUS << 48) |   /* PAT6: UC- */
>  ((uint64_t)PAT_TYPE_UNCACHABLE << 56);  /* PAT7: UC */
>  
> +if ( is_hardware_domain(v->domain) )
> +{
> +/* Copy values from the host. */
> +struct domain *d = v->domain;
> +unsigned int i;
> +
> +if ( mtrr_state.have_fixed )
> +for ( i = 0; i < NUM_FIXED_MSR; i++ )
> +mtrr_fix_range_msr_set(d, m, i,
> +  ((uint64_t 
> *)mtrr_state.fixed_ranges)[i]);
> +
> +for ( i = 0; i < num_var_ranges; i++ )
> +{
> +mtrr_var_range_msr_set(d, m, MSR_IA32_MTRR_PHYSBASE(i),
> +   mtrr_state.var_ranges[i].base);
> +mtrr_var_range_msr_set(d, m, MSR_IA32_MTRR_PHYSMASK(i),
> +   mtrr_state.var_ranges[i].mask);
> +}
> +
> +mtrr_def_type_msr_set(d, m,
> +  mtrr_state.def_type |
> +  (mtrr_state.enabled << MTRRdefType_FE_SHIFT));

This is all very clumsy (not your fault of course) - in particular, the 
"enabled"
field is a two-bit value. I'd rather not see this to continue to be that way,
and hence I've created a patch to clean this up (see below; I'm intentionally
retaining my own TODO notes in there for your reference, and it's also
unlikely to apply as is because it sits on top of other changes). In that light 
I
think I'd prefer if you either (later) re-based this onto my patch or reverted
back to the use of the literal 10 here.

Jan

TODO: rename _enabled to enabled for submission (but keep it this way here to
  make sure further possibly necessary adjustments are noticed)
TODO: replace 10 and 11 by manifest constants (Roger supposedly will add those)

x86/mtrr: split "enabled" field into two boolean flags

The code hopefully is more readable this way.

Also switch have_fixed to bool, seeing that it already is used as a
boolean.

Signed-off-by: Jan Beulich 

--- unstable.orig/xen/arch/x86/cpu/mtrr/generic.c
+++ unstable/xen/arch/x86/cpu/mtrr/generic.c
@@ -79,7 +79,8 @@ void __init get_mtrr_state(void)
 
rdmsrl(MSR_MTRRdefType, msr_content);
mtrr_state.def_type = (msr_content & 0xff);
-   mtrr_state.enabled = (msr_content & 0xc00) >> 10;
+   mtrr_state._enabled = (msr_content >> 11) & 1;
+   mtrr_state.fixed_enabled = (msr_content >> 10) & 1;
 
/* Store mtrr_cap for HVM MTRR virtualisation. */
rdmsrl(MSR_MTRRcap, mtrr_state.mtrr_cap);
@@ -158,7 +159,7 @@ static void __init print_mtrr_state(cons
unsigned int base = 0, step = 0x1;
 
printk("%sMTRR fixed ranges %sabled:\n", level,
-  mtrr_state.enabled & 1 ? "en" : "dis");
+  mtrr_state.fixed_enabled ? "en" : "dis");
for (; block->ranges; ++block, step >>= 2) {
for (i = 0; i < block->ranges; ++i, fr += 8) {
print_fixed(base, step, fr, level);
@@ -168,7 +169,7 @@ static void __init print_mtrr_state(cons
print_fixed_last(level);
}
printk("%sMTRR variable ranges %sabled:\n", level,
-  mtrr_state.enabled & 2 ? "en" : "dis");
+  mtrr_state._enabled ? "en" : "dis");
width = (paddr_bits - PAGE_SHIFT + 3) / 4;
 
for (i = 0; i < num_var_ranges; ++i) {
@@ -382,8 +383,11 @@ static unsigned long set_mtrr_state(void
/*  Set_mtrr_restore restores the old value of MTRRdefType,
   so to set it we fiddle with the saved value  */
if ((deftype & 0xff) != mtrr_state.def_type
-   || ((deftype & 0xc00) >> 10) != mtrr_state.enabled) {
-   deftype = (deftype & ~0xcff) | mtrr_state.def_type | 
(mtrr_state.enabled << 10);
+   || ((deftype >> 11) & 1) != mtrr_state._enabled
+   || ((deftype >> 10) & 1) != mtrr_state.fixed_enabled) {
+   deftype = (deftype & ~0xcff) | mtrr_state.def_type |
+ (mtrr_state._enabled << 11) |
+ (mtrr_state.fixed_enabled << 10);
change_mask |= MTRR_CHANGE_MASK_DEFTYPE;
}
 
--- unstable.orig/xen/arch/x86/hvm/hvm.c
+++ unstable/xen/arch/x86/hvm/hvm.c
@@ -3497,8 +3497,9 @@ int hvm_msr_read_intercept(unsigned int
 case MSR_MTRRdefType:
 if ( !d->arch.cpuid->basic.mtrr )
 goto gp_fault;
-*msr_content = v->arch.hvm_vcpu.mtrr.def_type
-| (v->arch.hvm_vcpu.mtrr.enabled << 10);
+*msr_content = v->arch.hvm_vcpu.mtrr.def_type |
+   (v->arch.hvm_vcpu.mtrr._enabled << 11) |
+   (v->arch.hvm_vcpu.mtrr.fixed_enabled << 

Re: [Xen-devel] [PATCH v2 3/6] hvm/mtrr: use the hardware number of variable ranges for Dom0

[Jan Beulich]

>>> On 15.05.18 at 16:36,  wrote:
> --- a/xen/arch/x86/hvm/mtrr.c
> +++ b/xen/arch/x86/hvm/mtrr.c
> @@ -154,14 +154,26 @@ uint8_t pat_type_2_pte_flags(uint8_t pat_type)
>  int hvm_vcpu_cacheattr_init(struct vcpu *v)
>  {
>  struct mtrr_state *m = >arch.hvm_vcpu.mtrr;
> +unsigned int num_var_ranges =
> +is_hardware_domain(v->domain) ? MASK_EXTR(mtrr_state.mtrr_cap,
> +  MTRRcap_VCNT)
> +  : MTRR_VCNT;
> +
> +if ( num_var_ranges > MTRR_VCNT_MAX )
> +{
> +ASSERT(is_hardware_domain(v->domain));
> +printk("WARNING: limited Dom0 variable range MTRRs from %u to %u\n",
> +   num_var_ranges, MTRR_VCNT_MAX);

Please log v->domain->domain_id here instead of hard coded Dom0, to cope
with the hardware domain being different from Dom0.

> @@ -675,6 +690,8 @@ static int hvm_save_mtrr_msr(struct domain *d, 
> hvm_domain_context_t *h)
>  /* save mtrr */
>  for_each_vcpu(d, v)
>  {
> +unsigned int num_var_ranges;
> +
>  mtrr_state = >arch.hvm_vcpu.mtrr;
>  
>  hvm_get_guest_pat(v, _mtrr.msr_pat_cr);
> @@ -683,7 +700,11 @@ static int hvm_save_mtrr_msr(struct domain *d, 
> hvm_domain_context_t *h)
>  | (mtrr_state->enabled << 10);
>  hw_mtrr.msr_mtrr_cap = mtrr_state->mtrr_cap;
>  
> -for ( i = 0; i < MTRR_VCNT; i++ )
> +num_var_ranges = MASK_EXTR(mtrr_state->mtrr_cap, MTRRcap_VCNT);
> +if ( num_var_ranges > MTRR_VCNT )
> +return -EINVAL;

This would better be accompanied by a dprintk().

> +for ( i = 0; i < num_var_ranges; i++ )

Following your v1 I had already put together a patch to change just the
save and load functions here, as the adjustments are necessary
independent of the Dom0 aspect. Should num_var_ranges indeed be
below MTRR_VCNT, there's an information leak here (of hypervisor stack
data) without pre-initializing hw_mtrr. Here's the hunk from my patch, in
case you care to re-use parts of it:

@@ -676,22 +676,22 @@ int hvm_set_mem_pinned_cacheattr(struct
 
 static int hvm_save_mtrr_msr(struct domain *d, hvm_domain_context_t *h)
 {
-int i;
 struct vcpu *v;
-struct hvm_hw_mtrr hw_mtrr;
-struct mtrr_state *mtrr_state;
+
 /* save mtrr */
 for_each_vcpu(d, v)
 {
-mtrr_state = >arch.hvm_vcpu.mtrr;
+const struct mtrr_state *mtrr_state = >arch.hvm_vcpu.mtrr;
+struct hvm_hw_mtrr hw_mtrr = {
+.msr_mtrr_def_type = mtrr_state->def_type |
+ (mtrr_state->enabled << 10),
+.msr_mtrr_cap  = mtrr_state->mtrr_cap,
+};
+unsigned int i;
 
 hvm_get_guest_pat(v, _mtrr.msr_pat_cr);
 
-hw_mtrr.msr_mtrr_def_type = mtrr_state->def_type
-| (mtrr_state->enabled << 10);
-hw_mtrr.msr_mtrr_cap = mtrr_state->mtrr_cap;
-
-for ( i = 0; i < MTRR_VCNT; i++ )
+for ( i = 0; i < (uint8_t)hw_mtrr.msr_mtrr_cap; i++ )
 {
 /* save physbase */
 hw_mtrr.msr_mtrr_var[i*2] =

(I didn't send it out yet as I'm generally of the opinion that prior to
branching focus should be on the code to be released rather than
the next following version.)

> --- a/xen/include/asm-x86/mtrr.h
> +++ b/xen/include/asm-x86/mtrr.h
> @@ -39,6 +39,8 @@ typedef u8 mtrr_type;
>  #define MTRR_PHYSBASE_SHIFT  12
>  /* Number of variable range MSR pairs we emulate for HVM guests: */
>  #define MTRR_VCNT8
> +/* Maximum number of variable range MSR pairs if FE is supported. */
> +#define MTRR_VCNT_MAX40

((MSR_MTRRfix64K_0 - MSR_IA32_MTRR_PHYSBASE(0)) / 2)

or some such please (to make obvious where the value is coming from).

Jan



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

Re: [Xen-devel] [PATCH v2 2/6] mtrr: introduce mask to get VCNT from MTRRcap MSR

[Jan Beulich]

>>> On 15.05.18 at 16:36,  wrote:
> @@ -478,7 +478,7 @@ bool_t mtrr_pat_not_equal(struct vcpu *vd, struct vcpu 
> *vs)
>  struct mtrr_state *md = >arch.hvm_vcpu.mtrr;
>  struct mtrr_state *ms = >arch.hvm_vcpu.mtrr;
>  int32_t res;
> -uint8_t num_var_ranges = (uint8_t)md->mtrr_cap;
> +uint8_t num_var_ranges = MASK_EXTR(md->mtrr_cap, MTRRcap_VCNT);

With the type changed to unsigned int here
Reviewed-by: Jan Beulich 
Of course this is easy enough to do while committing, should no other need
for a v3 arise.

Jan



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

Re: [Xen-devel] [PATCH v2 6/6] docs/pvh: document initial MTRR state

[Jan Beulich]

>>> On 15.05.18 at 16:36,  wrote:
> Provided to both Dom0 and DomUs.
> 
> Signed-off-by: Roger Pau Monné 
> ---
> 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 
> ---
> Changes since v1:
>  - Add an extra paragraph to clarify the initial MTRR state.
> ---
>  docs/misc/pvh.markdown | 18 ++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/docs/misc/pvh.markdown b/docs/misc/pvh.markdown
> index e85fb15374..fc31132618 100644
> --- a/docs/misc/pvh.markdown
> +++ b/docs/misc/pvh.markdown
> @@ -92,3 +92,21 @@ event channels. Delivery of those interrupts can be 
> configured in the same way
>  as HVM guests, check xen/include/public/hvm/params.h and
>  xen/include/public/hvm/hvm\_op.h for more information about available 
> delivery
>  methods.
> +
> +## MTRR ##
> +
> +### Unprivileged guests ###
> +
> +PVH guests are booted with the default MTRR type set to write-back and MTRR
> +enabled. This allows DomUs to start with a sane MTRR state. Note that this 
> will
> +have to be revisited when pci-passthrough is added to PVH in order to set 
> MMIO
> +regions as UC.
> +
> +Xen guarantees that RAM regions will always have the WB cache type set in the
> +initial MTRR state, either set by the default MTRR type or by other means.

While adding the second paragraph has helped, I still think the first one is 
lacking
"currently" or some such in the first sentence.

Jan


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

Re: [Xen-devel] [PATCH for-4.7/4.8] x86: Fix "x86: further CPUID handling adjustments"

[Jan Beulich]

>>> On 15.05.18 at 19:54,  wrote:
> Also, I don't see any link between the change and the commit message.  With
> the microcode installed, STIBP and IBPB are already visible to dom0.

They reportedly weren't (and I was able to confirm that), and given this
original (prior to that change) code

}
}
else
b = c = 0;
a = d = 0;

I also can't see how IBRSB and STIBP could have been visible. I agree I
had wrongly extended that to IBPB.

> The only required adjustment is to force STIBP == IBRSB, which must be done
> after applying the pv_featureset[] mask to the toolstack's choice of value.

I can see how I've got that part wrong from a leveling perspective (I was
really too focused on Dom0 back then), but I don't see how reporting IBPB
when IBRSB is available in hardware (implying IBPB itself isn't) would work
with your change in place.

I'm also not convinced assimilating Sergey's original change into this one is
appropriate - raw_featureset[] isn't used for anything except the sysctl.

Jan



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

Re: [Xen-devel] domain_crash_sync vs "plain crash"

[Wei Liu]

On Fri, May 11, 2018 at 07:43:58PM -0300, Charles Gonçalves wrote:
> "That is, without (physical
> or virtual, depending on component) serial console you're often unlikely to
> actually observe any messages connected to the crash."
> 
> I do not have any experience with serial console interaction on linux.
> Can you list some examples for both cases (virtual| physical), I'll glad to
> fully report any suspicious problem.
> 
> Thanks!
> 

See

https://wiki.xenproject.org/wiki/Xen_Serial_Console

Wei.

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

Re: [Xen-devel] [PATCH v1 for 4.7] x86/cpuid: fix raw FEATURESET_7d0 reporting

[Jan Beulich]

>>> On 15.05.18 at 17:36,  wrote:
> On 15/05/18 10:04, Jan Beulich wrote:
> On 15.05.18 at 10:37,  wrote:
>>> Commit 62b1879693e0 ("x86: further CPUID handling adjustments") added
>>> FEATURESET_7d0 reporting but forgot to update calculate_raw_featureset()
>>> function. As result, the value reported by xen-cpuid contains 0.
>>>
>>> Fix that by properly filling raw_featureset[FEATURESET_7d0].
>>>
>>> Signed-off-by: Sergey Dyasli 
>> Thanks, technically
>> Acked-by: Jan Beulich 
>>
>>> ---
>>> I see that at least 4.8 also contains this bug, so other releases also
>>> need checking.
>> The commit in question being only on the two branches, I think no other one
>> would need the change.
>>
>> I'm certainly going to apply this to 4.8; I'm uncertain about 4.7 though 
> 
> 
> The entirety of that changeset is broken in feature levelling scenarios,
> which is a consequence of missing this hunk which Sergey identified.
> 
> The result is that, when trying to level STIBP/IBPB out of guests view,
> the CPUID bits remain visible, but attempts to use the MSR bits will fail.

I don't follow: PV and HVM feature sets take host_featureset as an input,
not raw_featureset.

Jan



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

Re: [Xen-devel] [PATCH 06/10] x86/spec_ctrl: Split X86_FEATURE_SC_MSR into PV and HVM variants

[Jan Beulich]

>>> On 15.05.18 at 21:52,  wrote:
> On 14/05/18 16:27, Jan Beulich wrote:
> On 11.05.18 at 12:38,  wrote:
>>> --- a/xen/arch/x86/spec_ctrl.c
>>> +++ b/xen/arch/x86/spec_ctrl.c
>>> @@ -128,7 +128,8 @@ static void __init print_details(enum ind_thunk thunk, 
>>> uint64_t caps)
>>> thunk == THUNK_RETPOLINE ? "RETPOLINE" :
>>> thunk == THUNK_LFENCE? "LFENCE" :
>>> thunk == THUNK_JMP   ? "JMP" : "?",
>>> -   boot_cpu_has(X86_FEATURE_SC_MSR) ?
>>> +   (boot_cpu_has(X86_FEATURE_SC_MSR_PV) ||
>>> +boot_cpu_has(X86_FEATURE_SC_MSR_HVM)) ?
>>> default_xen_spec_ctrl & SPEC_CTRL_IBRS? " IBRS+" :
>>> " IBRS-"  : "",
>>> opt_ibpb  ? " IBPB"   : "",
>>> @@ -367,7 +368,8 @@ void __init init_speculation_mitigations(void)
>>>   * need the IBRS entry/exit logic to virtualise IBRS support for
>>>   * guests.
>>>   */
>>> -setup_force_cpu_cap(X86_FEATURE_SC_MSR);
>>> +setup_force_cpu_cap(X86_FEATURE_SC_MSR_PV);
>>> +setup_force_cpu_cap(X86_FEATURE_SC_MSR_HVM);
>> Besides these sort of open coding alternative_io_2() (you'd really want an
>> output-less variant here, I agree) these are slightly bending the rules of
>> when/how to use multiple alternatives: The above ends up correct only
>> because of both replacements being identical.
> 
> Actually, by reordering patch 10 ahead of this patch, we never get to
> needing the ALTERNATIVE_2()'s in the first place, and lose any concerns
> with bending the rules along the series.

Ah yes, indeed. And you would better use alternative_input() there then,
instead of open coding it.

Jan



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