Re: Problems starting Xen domU after latest stable update

[Jason Andryuk]

On Sat, Jan 30, 2021 at 6:25 PM Marek Marczykowski-Górecki
 wrote:
>
> On Fri, Jan 29, 2021 at 03:16:52PM +0100, Jürgen Groß wrote:
> > On 29.01.21 15:13, Michael Labriola wrote:
> > > On Fri, Jan 29, 2021 at 12:26 AM Jürgen Groß  wrote:
> > > > If the buggy patch has been put into stable this Fixes: tag should
> > > > result in the fix being put into the same stable branches as well.
> > >
> > > I've never done this before...  does this happen automatically?  Or is
> > > there somebody we should ping to make sure it happens?
> >
> > This happens automatically (I think).
> >
> > I have seen mails for the patch been taken for 4.14, 4.19, 5.4 and 5.10.
>
> Hmm, I can't find it in LKML archive, nor stable@ archive. And also it
> isn't included in 5.10.12 released yesterday, nor included in
> queue/5.10 [1]. Are you sure it wasn't lost somewhere in the meantime?

It probably would have gotten in eventually, but I made the explicit
request to move things along.

-Jason

[PATCH] xen: Kconfig: remove X86_64 depends from XEN_512GB

[Jason Andryuk]

commit bfda93aee0ec ("xen: Kconfig: nest Xen guest options")
accidentally re-added X86_64 as a dependency to XEN_512GB.  It was
originally removed in commit a13f2ef168cb ("x86/xen: remove 32-bit Xen
PV guest support").  Remove it again.

Fixes: bfda93aee0ec ("xen: Kconfig: nest Xen guest options")
Reported-by: Boris Ostrovsky 
Signed-off-by: Jason Andryuk 
---
This fixes Boris's review comment.  I lost track of posting a v2 with it
fixed.
---
 arch/x86/xen/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/xen/Kconfig b/arch/x86/xen/Kconfig
index 2b105888927c..afc1da68b06d 100644
--- a/arch/x86/xen/Kconfig
+++ b/arch/x86/xen/Kconfig
@@ -28,7 +28,7 @@ config XEN_PV
 
 config XEN_512GB
bool "Limit Xen pv-domain memory to 512GB"
-   depends on XEN_PV && X86_64
+   depends on XEN_PV
default y
help
  Limit paravirtualized user domains to 512GB of RAM.
-- 
2.29.2

Re: [PATCH 2/2] xen: don't use page->lru for ZONE_DEVICE memory

[Jason Andryuk]

On Thu, Dec 10, 2020 at 6:40 AM Jürgen Groß  wrote:
>
> On 10.12.20 12:14, Roger Pau Monné wrote:
> > On Tue, Dec 08, 2020 at 07:45:00AM +0100, Jürgen Groß wrote:
> >> On 07.12.20 21:48, Jason Andryuk wrote:
> >>> On Mon, Dec 7, 2020 at 8:30 AM Juergen Gross  wrote:
> >>>>
> >>>> Commit 9e2369c06c8a18 ("xen: add helpers to allocate unpopulated
> >>>> memory") introduced usage of ZONE_DEVICE memory for foreign memory
> >>>> mappings.
> >>>>
> >>>> Unfortunately this collides with using page->lru for Xen backend
> >>>> private page caches.
> >>>>
> >>>> Fix that by using page->zone_device_data instead.
> >>>>
> >>>> Fixes: 9e2369c06c8a18 ("xen: add helpers to allocate unpopulated memory")
> >>>> Signed-off-by: Juergen Gross 
> >>>
> >>> Would it make sense to add BUG_ON(is_zone_device_page(page)) and the
> >>> opposite as appropriate to cache_enq?
> >>
> >> No, I don't think so. At least in the CONFIG_ZONE_DEVICE case the
> >> initial list in a PV dom0 is populated from extra memory (basically
> >> the same, but not marked as zone device memory explicitly).
> >
> > I assume it's fine for us to then use page->zone_device_data even if
> > the page is not explicitly marked as ZONE_DEVICE memory?
>
> I think so, yes, as we are owner of that page and we were fine to use
> lru, too.

I think memremap_pages or devm_memremap_pages (which calls
memremap_pages) is how you mark memory as ZONE_DEVICE.  i.e. they are
explicitly marked.

memremap_pages
  memmap_init_zone_device (with ZONE_DEVICE)
__init_single_page
  set_page_links
set_page_zone

grep only finds a few uses of ZONE_DEVICE in the whole tree.

Regards,
Jason

Re: [PATCH 2/2] xen: don't use page->lru for ZONE_DEVICE memory

[Jason Andryuk]

On Mon, Dec 7, 2020 at 8:30 AM Juergen Gross  wrote:
>
> Commit 9e2369c06c8a18 ("xen: add helpers to allocate unpopulated
> memory") introduced usage of ZONE_DEVICE memory for foreign memory
> mappings.
>
> Unfortunately this collides with using page->lru for Xen backend
> private page caches.
>
> Fix that by using page->zone_device_data instead.
>
> Fixes: 9e2369c06c8a18 ("xen: add helpers to allocate unpopulated memory")
> Signed-off-by: Juergen Gross 

Would it make sense to add BUG_ON(is_zone_device_page(page)) and the
opposite as appropriate to cache_enq?  Either way:
Reviewed-by: Jason Andryuk 

Re: [PATCH] i915: Add QUIRK_EDP_CHANNEL_EQ for Dell 7200 2-in-1

[Jason Andryuk]

On Fri, Oct 23, 2020 at 8:48 AM Jason Andryuk  wrote:
>
> We're seeing channel equalization "fail" consistently coming out of DPMS
> on the eDP of a Dell Latitude 7200 2-in-1.  When the display tries to
> come out of DPMS, it briefly flashes on before going dark.  This repeats
> once per second, and the system is unusable.  ssh-ing into the system,
> it also seems to be sluggish when in this state.  You have to reboot to
> get the display back.
>
> In intel_dp_link_training_channel_equalization, lane 0 can get to state
> 0x7 by the 3rd pattern, but lane 1 never gets further than 0x1.
> [drm] ln0_1:0x0 ln2_3:0x0 align:0x0 sink:0x0 adj_req0_1:0x0 adj_req2_3:0x0
> [drm] ln0_1:0x11 ln2_3:0x0 align:0x80 sink:0x0 adj_req0_1:0x44 adj_req2_3:0x0
> [drm] ln0_1:0x11 ln2_3:0x0 align:0x80 sink:0x0 adj_req0_1:0x88 adj_req2_3:0x0
> [drm] ln0_1:0x71 ln2_3:0x0 align:0x80 sink:0x0 adj_req0_1:0x84 adj_req2_3:0x0
> [drm] ln0_1:0x71 ln2_3:0x0 align:0x0 sink:0x0 adj_req0_1:0x84 adj_req2_3:0x0
> [drm] ln0_1:0x71 ln2_3:0x0 align:0x0 sink:0x0 adj_req0_1:0x84 adj_req2_3:0x0
>
> Narrow fast vs. wide slow is not an option because the max clock is
> 27 and the 1920x1280 resolution requires 2x27.
> [drm] DP link computation with lane count min/max 1/2 27/27 bpp
> min/max 18/24 pixel clock 164250KHz
>
> The display is functional even though lane 1 is in state 0x1, so just
> return success for channel equalization on eDP.
>
> Introduce QUIRK_EDP_CHANNEL_EQ and match the DMI for a Dell Latitude
> 7200 2-in-1.  This quirk allows channel equalization to succeed even
> though it failed.
>
> Workaround for https://gitlab.freedesktop.org/drm/intel/-/issues/1378

CI reported the patch doesn't apply to drm-tip.  It was developed
against 5.4 and forward ported to 5.10-rc1-ish when I submitted it.
It applied there but not to drm-tip.

5.4 & 5.6.6 is fine until DPMS.  Then when it tries to come out, it
fails link training and gives the flashing.
5.8.16 starts flashing during boot.  I guess the driver now runs link
training during boot?

drm-tip doesn't have the flashing issue.  I guess "drm/i915: Switch to
LTTPR non-transparent mode link training"  or some of the other link
training change lets the hardware succeed?

Oh, this is interesting:
kernel: i915 :00:02.0: [drm:hsw_set_signal_levels [i915]] Using
signal levels 0200
kernel: [drm:intel_dp_link_train_phy [i915]] ln0_1:0x71 ln2_3:0x0
align:0x0 sink:0x0 adj_req0_1:0x84 adj_req2_3:0x0
kernel: i915 :00:02.0: [drm:intel_dp_link_train_phy [i915]]
Channel equalization failed 5 times
kernel: i915 :00:02.0: [drm:intel_dp_link_train_phy [i915]]
[CONNECTOR:95:eDP-1] Link Training failed at link rate = 27, lane
count = 2, at DPRX
kernel: i915 :00:02.0: [drm:intel_enable_pipe [i915]] enabling pipe A

Note DPRX above, so not using LTTPR.

Looks like the link training logic is wrong. :

intel_dp_link_training_channel_equalization fails, so
intel_dp_link_train_phy fails, but:

intel_dp_link_train_all_phys(struct intel_dp *intel_dp,
 const struct intel_crtc_state *crtc_state,
 int lttpr_count)
{
bool ret = true;
int i;

intel_dp_prepare_link_train(intel_dp, crtc_state);

for (i = lttpr_count - 1; i >= 0; i--) {
enum drm_dp_phy dp_phy = DP_PHY_LTTPR(i);

ret = intel_dp_link_train_phy(intel_dp, crtc_state, dp_phy);
intel_dp_disable_dpcd_training_pattern(intel_dp, dp_phy);

if (!ret)
break;
}

if (ret)
intel_dp_link_train_phy(intel_dp, crtc_state, DP_PHY_DPRX);

Here we don't update ret, so linking training "succeeds" for DPRX.

This does let the 7200 display "work", but it's probably not what you
want in general.

Regards,
Jason

[PATCH] i915: Add QUIRK_EDP_CHANNEL_EQ for Dell 7200 2-in-1

[Jason Andryuk]

We're seeing channel equalization "fail" consistently coming out of DPMS
on the eDP of a Dell Latitude 7200 2-in-1.  When the display tries to
come out of DPMS, it briefly flashes on before going dark.  This repeats
once per second, and the system is unusable.  ssh-ing into the system,
it also seems to be sluggish when in this state.  You have to reboot to
get the display back.

In intel_dp_link_training_channel_equalization, lane 0 can get to state
0x7 by the 3rd pattern, but lane 1 never gets further than 0x1.
[drm] ln0_1:0x0 ln2_3:0x0 align:0x0 sink:0x0 adj_req0_1:0x0 adj_req2_3:0x0
[drm] ln0_1:0x11 ln2_3:0x0 align:0x80 sink:0x0 adj_req0_1:0x44 adj_req2_3:0x0
[drm] ln0_1:0x11 ln2_3:0x0 align:0x80 sink:0x0 adj_req0_1:0x88 adj_req2_3:0x0
[drm] ln0_1:0x71 ln2_3:0x0 align:0x80 sink:0x0 adj_req0_1:0x84 adj_req2_3:0x0
[drm] ln0_1:0x71 ln2_3:0x0 align:0x0 sink:0x0 adj_req0_1:0x84 adj_req2_3:0x0
[drm] ln0_1:0x71 ln2_3:0x0 align:0x0 sink:0x0 adj_req0_1:0x84 adj_req2_3:0x0

Narrow fast vs. wide slow is not an option because the max clock is
27 and the 1920x1280 resolution requires 2x27.
[drm] DP link computation with lane count min/max 1/2 27/27 bpp
min/max 18/24 pixel clock 164250KHz

The display is functional even though lane 1 is in state 0x1, so just
return success for channel equalization on eDP.

Introduce QUIRK_EDP_CHANNEL_EQ and match the DMI for a Dell Latitude
7200 2-in-1.  This quirk allows channel equalization to succeed even
though it failed.

Workaround for https://gitlab.freedesktop.org/drm/intel/-/issues/1378

Signed-off-by: Jason Andryuk 
Cc: 
---
 .../drm/i915/display/intel_dp_link_training.c |  7 +
 drivers/gpu/drm/i915/display/intel_quirks.c   | 30 +++
 drivers/gpu/drm/i915/i915_drv.h   |  1 +
 3 files changed, 38 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c 
b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
index a23ed7290843..2dd441a94fda 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -375,6 +375,13 @@ intel_dp_link_training_channel_equalization(struct 
intel_dp *intel_dp)
 
intel_dp_set_idle_link_train(intel_dp);
 
+   if (channel_eq == false &&
+   intel_dp_is_edp(intel_dp) &&
+   i915->quirks & QUIRK_EDP_CHANNEL_EQ) {
+   DRM_NOTE("Forcing channel_eq success for eDP\n");
+   channel_eq = true;
+   }
+
return channel_eq;
 
 }
diff --git a/drivers/gpu/drm/i915/display/intel_quirks.c 
b/drivers/gpu/drm/i915/display/intel_quirks.c
index 46beb155d835..b45b23680321 100644
--- a/drivers/gpu/drm/i915/display/intel_quirks.c
+++ b/drivers/gpu/drm/i915/display/intel_quirks.c
@@ -53,6 +53,17 @@ static void quirk_increase_ddi_disabled_time(struct 
drm_i915_private *i915)
drm_info(>drm, "Applying Increase DDI Disabled quirk\n");
 }
 
+/*
+ * Some machines (Dell Latitude 7200 2-in-1) fail channel equalization
+ * on their eDP when it is actually usable.  This lets channel_eq
+ * report success.
+ */
+static void quirk_edp_channel_eq(struct drm_i915_private *i915)
+{
+   i915->quirks |= QUIRK_EDP_CHANNEL_EQ;
+   drm_info(>drm, "applying eDP channel_eq quirk\n");
+}
+
 struct intel_quirk {
int device;
int subsystem_vendor;
@@ -72,6 +83,12 @@ static int intel_dmi_reverse_brightness(const struct 
dmi_system_id *id)
return 1;
 }
 
+static int intel_dmi_edp_channel_eq(const struct dmi_system_id *id)
+{
+   DRM_INFO("eDP channel_eq workaround on %s\n", id->ident);
+   return 1;
+}
+
 static const struct intel_dmi_quirk intel_dmi_quirks[] = {
{
.dmi_id_list = &(const struct dmi_system_id[]) {
@@ -96,6 +113,19 @@ static const struct intel_dmi_quirk intel_dmi_quirks[] = {
},
.hook = quirk_invert_brightness,
},
+   {
+   .dmi_id_list = &(const struct dmi_system_id[]) {
+   {
+   .callback = intel_dmi_edp_channel_eq,
+   .ident = "Dell Latitude 7200 2-in-1",
+   .matches = {DMI_MATCH(DMI_SYS_VENDOR, "Dell 
Inc."),
+   DMI_MATCH(DMI_PRODUCT_NAME, 
"Latitude 7200 2-in-1"),
+   },
+   },
+   { }  /* terminating entry */
+   },
+   .hook = quirk_edp_channel_eq,
+   },
 };
 
 static struct intel_quirk intel_quirks[] = {
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e4f7f6518945..fc32ea7380b7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -525,6 +525,7 @@ struct i915_psr {
 #define QUIRK_PIN_SWIZZLED_PAGES (1<<5)
 #defin

Re: [PATCH] hid-mt: Fix Cirque 121f touch release

[Jason Andryuk]

On Mon, Oct 19, 2020 at 9:05 AM Jason Andryuk  wrote:
>
> We're seeing the touchpad not send all the touch release events.  This
> can result in the cursor getting stuck generating scroll events instead
> of cursor movement for single finger motion.  With the cursor not
> moving, users think the trackpad is broken.  With libinput-record, you
> can see that it doesn't always get to a neutral state when there are no
> fingers on the touchpad.
>
> MT_QUIRK_STICKY_FINGERS was insufficient alone.  The timer often didn't
> fire to release all the contacts.  MT_QUIRK_NOT_SEEN_MEANS_UP seems to
> help with tracking the touches, and allows the timer to fire properly
> when needed.
>
> You can reproduce by touching the trackpad with 4 fingers spread out,
> then pulling them all together and removing from the track pad.



> ---
> This is developed and tested against 5.4 and forward ported to latest
> upstream.

I installed Fedora 32 with kernel 5.6.6 to test out some other stuff,
and it is not reproducing with the steps above.  Is there some other
change that may have fixed the release tracking?  I was definitely
seeing it with 5.4.72.

Regards,
Jason

[PATCH] hid-mt: Fix Cirque 121f touch release

[Jason Andryuk]

We're seeing the touchpad not send all the touch release events.  This
can result in the cursor getting stuck generating scroll events instead
of cursor movement for single finger motion.  With the cursor not
moving, users think the trackpad is broken.  With libinput-record, you
can see that it doesn't always get to a neutral state when there are no
fingers on the touchpad.

MT_QUIRK_STICKY_FINGERS was insufficient alone.  The timer often didn't
fire to release all the contacts.  MT_QUIRK_NOT_SEEN_MEANS_UP seems to
help with tracking the touches, and allows the timer to fire properly
when needed.

You can reproduce by touching the trackpad with 4 fingers spread out,
then pulling them all together and removing from the track pad.

libinput-record output without the patch:
  - evdev:
- [  2, 591028,   4,   5, 2561700] # EV_MSC / MSC_TIMESTAMP2561700
- [  2, 591028,   0,   0,   0] #  SYN_REPORT (0) -- 
+7ms
  - evdev:
- [  2, 603307,   3,  57,  -1] # EV_ABS / ABS_MT_TRACKING_ID   -1
- [  2, 603307,   1, 334,   1] # EV_KEY / BTN_TOOL_TRIPLETAP1
- [  2, 603307,   1, 335,   0] # EV_KEY / BTN_TOOL_QUADTAP  0
- [  2, 603307,   4,   5, 2570700] # EV_MSC / MSC_TIMESTAMP2570700
- [  2, 603307,   0,   0,   0] #  SYN_REPORT (0) -- 
+12ms

Note the lack of "# Touch device in neutral state".  There has been no
activity for 2 seconds, but the tripletap has not cleared.  If you place
a finger back on at this point, it gets interpreted as a quadtap.

With this patch:
  - evdev:
- [1123, 442602,   3,  47,   3] # EV_ABS / ABS_MT_SLOT   3
- [1123, 442602,   3,  54, 133] # EV_ABS / ABS_MT_POSITION_Y   133 
(+4)
- [1123, 442602,   3,  47,   2] # EV_ABS / ABS_MT_SLOT   2
- [1123, 442602,   3,  53, 667] # EV_ABS / ABS_MT_POSITION_X   667 
(-2)
- [1123, 442602,   3,  54, 121] # EV_ABS / ABS_MT_POSITION_Y   121 
(+4)
- [1123, 442602,   3,  47,   1] # EV_ABS / ABS_MT_SLOT   1
- [1123, 442602,   3,  57,  -1] # EV_ABS / ABS_MT_TRACKING_ID   -1
- [1123, 442602,   1, 333,   1] # EV_KEY / BTN_TOOL_DOUBLETAP1
- [1123, 442602,   1, 334,   0] # EV_KEY / BTN_TOOL_TRIPLETAP0
- [1123, 442602,   3,   0, 667] # EV_ABS / ABS_X   667 
(+321)
- [1123, 442602,   3,   1, 121] # EV_ABS / ABS_Y   121 
(-204)
- [1123, 442602,   4,   5,  196500] # EV_MSC / MSC_TIMESTAMP196500
- [1123, 442602,   0,   0,   0] #  SYN_REPORT (0) 
-- +5ms
  - evdev:
- [1123, 544680,   3,  47,   2] # EV_ABS / ABS_MT_SLOT   2
- [1123, 544680,   3,  57,  -1] # EV_ABS / ABS_MT_TRACKING_ID   -1
- [1123, 544680,   3,  47,   3] # EV_ABS / ABS_MT_SLOT   3
- [1123, 544680,   3,  57,  -1] # EV_ABS / ABS_MT_TRACKING_ID   -1
- [1123, 544680,   1, 330,   0] # EV_KEY / BTN_TOUCH 0
- [1123, 544680,   1, 333,   0] # EV_KEY / BTN_TOOL_DOUBLETAP0
- [1123, 544680,   0,   0,   0] #  SYN_REPORT (0) 
-- +102ms
 # Touch device in neutral state

Here the timer fired and cleared the lingering doubletap.

Create a new MT_CLS_CIRQUE by copying MT_CLS_WIN_8 and replacing
MT_QUIRK_ALWAYS_VALID with MT_QUIRK_NOT_SEEN_MEANS_UP
(MT_QUIRK_STICKY_FINGERS is already in MT_CLS_WIN_8).

Signed-off-by: Jason Andryuk 
Cc: 

---
This is developed and tested against 5.4 and forward ported to latest
upstream.

I don't know if it needs to be quirked more specifically, but the
modalias is:
input:b0018v0488p121Fe0100-e0,1,3,4,k110,111,145,148,14A,14D,14E,14F,ra0,1,2F,35,36,37,39,m5,lsfw

I don't know if MT_CLS_CIRQUE needs the HID_DG_CONFIDENCE change, but it
is included to keep it as close as possible to MT_CLS_WIN_8.

I opened this bug originally with some libinput debugging info:
https://gitlab.freedesktop.org/libinput/libinput/-/issues/531

This might be related:
https://bugs.launchpad.net/dell-sputnik/+bug/1651635

With this patch I occasionally see an MSC_TIMESTAMP/SYN_REPORT come
through after "# Touch device in neutral stat".  Is that a problem, or
will it just be ignored?
---
 drivers/hid/hid-ids.h|  3 +++
 drivers/hid/hid-multitouch.c | 18 +-
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 74fc1df6e3c2..7e153cf3ab16 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -278,6 +278,9 @@
 
 #define USB_VENDOR_ID_CIDC 0x1677
 
+#define I2C_VENDOR_ID_CIRQUE   0x0488
+#define I2C_PRODUCT_ID_CIRQUE_121F 0x121F
+
 #define USB_VENDOR_ID_CJTOUCH  0x24b8
 #define USB_DEVICE_ID_CJTOUCH_MULTI_TOUCH_0020 0x0020
 #define USB_DEVICE_ID_CJTOUCH_MULTI_TOUCH_00

Re: [PATCH 1/2] xen: Remove Xen PVH/PVHVM dependency on PCI

[Jason Andryuk]

On Thu, Oct 15, 2020 at 4:10 AM Jan Beulich  wrote:
>
> On 14.10.2020 19:53, Jason Andryuk wrote:
> > @@ -76,7 +80,9 @@ config XEN_DEBUG_FS
> > Enabling this option may incur a significant performance overhead.
> >
> >  config XEN_PVH
> > - bool "Support for running as a Xen PVH guest"
> > + bool "Xen PVH guest support"
>
> Tangential question: Is "guest" here still appropriate, i.e.
> isn't this option also controlling whether the kernel can be
> used in a PVH Dom0?

Would you like something more generic like "Xen PVH support" and
"Support for running in Xen PVH mode"?

> >   def_bool n
>
> And is this default still appropriate?

We probably want to flip it on, yes.  PVH is the future, isn't it?

Regards,
Jason

Re: [PATCH 2/2] xen: Kconfig: nest Xen guest options

[Jason Andryuk]

On Thu, Oct 15, 2020 at 5:42 AM Jürgen Groß  wrote:
>
> On 14.10.20 19:53, Jason Andryuk wrote:
> > Moving XEN_512GB allows it to nest under XEN_PV.  That also allows
> > XEN_PVH to nest under XEN as a sibling to XEN_PV and XEN_PVHVM giving:
> >
> > [*]   Xen guest support
> > [*] Xen PV guest support
> > [*]   Limit Xen pv-domain memory to 512GB
> > [*]   Xen PV Dom0 support
>
> This has currently a wrong text/semantics:
>
> It should be split to CONFIG_XEN_DOM0 and CONFIG_XEN_PV_DOM0.
>
> Otherwise the backends won't be enabled per default for a PVH-only
> config meant to be Dom0-capable.
>
> You don't have to do that in your patches if you don't want to, but
> I wanted to mention it with you touching this area of Kconfig.

Yes, good point.  I had not considered that.

> > [*]     Xen PVHVM guest support
> > [*] Xen PVH guest support
> >
> > Signed-off-by: Jason Andryuk 
>
> Reviewed-by: Juergen Gross 

Thanks,
Jason

Re: [PATCH 2/2] xen: Kconfig: nest Xen guest options

[Jason Andryuk]

On Thu, Oct 15, 2020 at 9:17 AM  wrote:
>
>
> On 10/15/20 9:10 AM, Andrew Cooper wrote:
> > On 15/10/2020 13:37, boris.ostrov...@oracle.com wrote:
> >> On 10/14/20 1:53 PM, Jason Andryuk wrote:
> >>> +config XEN_512GB
> >>> +   bool "Limit Xen pv-domain memory to 512GB"
> >>> +   depends on XEN_PV && X86_64
> >> Why is X86_64 needed here?
> >> 512G support was implemented using a direct-mapped P2M, and is rather
> > beyond the virtual address capabilities of 32bit.
> >
>
> Yes, my point was that XEN_PV already depends on X86_64.

Oh, thanks for catching this.  I re-introduced it by accident when
rebasing the patches.

Regards,
Jason

Re: [SUSPECTED SPAM][PATCH 0/2] Remove Xen PVH dependency on PCI

[Jason Andryuk]

On Wed, Oct 14, 2020 at 2:04 PM Andrew Cooper  wrote:
>
> On 14/10/2020 18:53, Jason Andryuk wrote:
> > A Xen PVH domain doesn't have a PCI bus or devices,
>
> [*] Yet.

:)

> > so it doesn't need PCI support built in.
>
> Untangling the dependences is a good thing, but eventually we plan to
> put an optional PCI bus back in, e.g. for SRIOV usecases.

Yes, and to be clear this change doesn't preclude including the PCI
code.  I was just looking to remove code from my VMs that aren't using
PCI devices.

Regards,
Jason

[PATCH 0/2] Remove Xen PVH dependency on PCI

[Jason Andryuk]

A Xen PVH domain doesn't have a PCI bus or devices, so it doesn't need
PCI support built in.  Currently, XEN_PVH depends on XEN_PVHVM which
depends on PCI.

The first patch introduces XEN_PVHVM_GUEST as a toplevel item and
changes XEN_PVHVM to a hidden variable.  This allows XEN_PVH to depend
on XEN_PVHVM without PCI while XEN_PVHVM_GUEST depends on PCI.

The second patch moves XEN_512GB to clean up the option nesting.

Jason Andryuk (2):
  xen: Remove Xen PVH/PVHVM dependency on PCI
  xen: Kconfig: nest Xen guest options

 arch/x86/xen/Kconfig | 38 ++
 drivers/xen/Makefile |  2 +-
 2 files changed, 23 insertions(+), 17 deletions(-)

-- 
2.26.2

[PATCH 2/2] xen: Kconfig: nest Xen guest options

[Jason Andryuk]

Moving XEN_512GB allows it to nest under XEN_PV.  That also allows
XEN_PVH to nest under XEN as a sibling to XEN_PV and XEN_PVHVM giving:

[*]   Xen guest support
[*] Xen PV guest support
[*]   Limit Xen pv-domain memory to 512GB
[*]   Xen PV Dom0 support
[*] Xen PVHVM guest support
[*] Xen PVH guest support

Signed-off-by: Jason Andryuk 
---
 arch/x86/xen/Kconfig | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/arch/x86/xen/Kconfig b/arch/x86/xen/Kconfig
index b75007eb4ec4..2b105888927c 100644
--- a/arch/x86/xen/Kconfig
+++ b/arch/x86/xen/Kconfig
@@ -26,6 +26,19 @@ config XEN_PV
help
  Support running as a Xen PV guest.
 
+config XEN_512GB
+   bool "Limit Xen pv-domain memory to 512GB"
+   depends on XEN_PV && X86_64
+   default y
+   help
+ Limit paravirtualized user domains to 512GB of RAM.
+
+ The Xen tools and crash dump analysis tools might not support
+ pv-domains with more than 512 GB of RAM. This option controls the
+ default setting of the kernel to use only up to 512 GB or more.
+ It is always possible to change the default via specifying the
+ boot parameter "xen_512gb_limit".
+
 config XEN_PV_SMP
def_bool y
depends on XEN_PV && SMP
@@ -53,19 +66,6 @@ config XEN_PVHVM_GUEST
help
  Support running as a Xen PVHVM guest.
 
-config XEN_512GB
-   bool "Limit Xen pv-domain memory to 512GB"
-   depends on XEN_PV
-   default y
-   help
- Limit paravirtualized user domains to 512GB of RAM.
-
- The Xen tools and crash dump analysis tools might not support
- pv-domains with more than 512 GB of RAM. This option controls the
- default setting of the kernel to use only up to 512 GB or more.
- It is always possible to change the default via specifying the
- boot parameter "xen_512gb_limit".
-
 config XEN_SAVE_RESTORE
bool
depends on XEN
-- 
2.26.2

[PATCH 1/2] xen: Remove Xen PVH/PVHVM dependency on PCI

[Jason Andryuk]

A Xen PVH domain doesn't have a PCI bus or devices, so it doesn't need
PCI support built in.  Currently, XEN_PVH depends on XEN_PVHVM which
depends on PCI.

Introduce XEN_PVHVM_GUEST as a toplevel item and change XEN_PVHVM to a
hidden variable.  This allows XEN_PVH to depend on XEN_PVHVM without PCI
while XEN_PVHVM_GUEST depends on PCI.

In drivers/xen, compile platform-pci depending on XEN_PVHVM_GUEST since
that pulls in the PCI dependency for linking.

Signed-off-by: Jason Andryuk 
---
---
 arch/x86/xen/Kconfig | 18 --
 drivers/xen/Makefile |  2 +-
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/arch/x86/xen/Kconfig b/arch/x86/xen/Kconfig
index 218acbd5c7a0..b75007eb4ec4 100644
--- a/arch/x86/xen/Kconfig
+++ b/arch/x86/xen/Kconfig
@@ -39,16 +39,20 @@ config XEN_DOM0
  Support running as a Xen PV Dom0 guest.
 
 config XEN_PVHVM
-   bool "Xen PVHVM guest support"
-   default y
-   depends on XEN && PCI && X86_LOCAL_APIC
-   help
- Support running as a Xen PVHVM guest.
+   def_bool y
+   depends on XEN && X86_LOCAL_APIC
 
 config XEN_PVHVM_SMP
def_bool y
depends on XEN_PVHVM && SMP
 
+config XEN_PVHVM_GUEST
+   bool "Xen PVHVM guest support"
+   default y
+   depends on XEN_PVHVM && PCI
+   help
+ Support running as a Xen PVHVM guest.
+
 config XEN_512GB
bool "Limit Xen pv-domain memory to 512GB"
depends on XEN_PV
@@ -76,7 +80,9 @@ config XEN_DEBUG_FS
  Enabling this option may incur a significant performance overhead.
 
 config XEN_PVH
-   bool "Support for running as a Xen PVH guest"
+   bool "Xen PVH guest support"
depends on XEN && XEN_PVHVM && ACPI
select PVH
def_bool n
+   help
+ Support for running as a Xen PVH guest.
diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
index babdca808861..c3621b9f4012 100644
--- a/drivers/xen/Makefile
+++ b/drivers/xen/Makefile
@@ -21,7 +21,7 @@ obj-$(CONFIG_XEN_GNTDEV)  += xen-gntdev.o
 obj-$(CONFIG_XEN_GRANT_DEV_ALLOC)  += xen-gntalloc.o
 obj-$(CONFIG_XENFS)+= xenfs/
 obj-$(CONFIG_XEN_SYS_HYPERVISOR)   += sys-hypervisor.o
-obj-$(CONFIG_XEN_PVHVM)+= platform-pci.o
+obj-$(CONFIG_XEN_PVHVM_GUEST)  += platform-pci.o
 obj-$(CONFIG_SWIOTLB_XEN)  += swiotlb-xen.o
 obj-$(CONFIG_XEN_MCE_LOG)  += mcelog.o
 obj-$(CONFIG_XEN_PCIDEV_BACKEND)   += xen-pciback/
-- 
2.26.2

Re: [PATCH 03/13] x86: Add early SHA support for Secure Launch early measurements

[Jason Andryuk]

On Thu, Sep 24, 2020 at 11:00 AM Ross Philipson
 wrote:
>
> The SHA algorithms are necessary to measure configuration information into
> the TPM as early as possible before using the values. This implementation
> uses the established approach of #including the SHA libraries directly in
> the code since the compressed kernel is not uncompressed at this point.
>
> The SHA code here has its origins in the code from the main kernel. That
> code could not be pulled directly into the setup portion of the compressed
> kernel because of other dependencies it pulls in. The result is this is a
> modified copy of that code that still leverages the core SHA algorithms.
>
> Signed-off-by: Daniel P. Smith 
> Signed-off-by: Ross Philipson 
> ---
>  arch/x86/boot/compressed/Makefile   |   4 +
>  arch/x86/boot/compressed/early_sha1.c   | 104 
>  arch/x86/boot/compressed/early_sha1.h   |  17 +++
>  arch/x86/boot/compressed/early_sha256.c |   6 +
>  arch/x86/boot/compressed/early_sha512.c |   6 +
>  include/linux/sha512.h  |  21 
>  lib/sha1.c  |   4 +
>  lib/sha512.c| 209 
> 
>  8 files changed, 371 insertions(+)
>  create mode 100644 arch/x86/boot/compressed/early_sha1.c
>  create mode 100644 arch/x86/boot/compressed/early_sha1.h
>  create mode 100644 arch/x86/boot/compressed/early_sha256.c
>  create mode 100644 arch/x86/boot/compressed/early_sha512.c
>  create mode 100644 include/linux/sha512.h
>  create mode 100644 lib/sha512.c
>
> diff --git a/arch/x86/boot/compressed/Makefile 
> b/arch/x86/boot/compressed/Makefile
> index ff7894f..0fd84b9 100644
> --- a/arch/x86/boot/compressed/Makefile
> +++ b/arch/x86/boot/compressed/Makefile
> @@ -96,6 +96,10 @@ vmlinux-objs-$(CONFIG_ACPI) += $(obj)/acpi.o
>  vmlinux-objs-$(CONFIG_EFI_MIXED) += $(obj)/efi_thunk_$(BITS).o
>  efi-obj-$(CONFIG_EFI_STUB) = $(objtree)/drivers/firmware/efi/libstub/lib.a
>
> +vmlinux-objs-$(CONFIG_SECURE_LAUNCH) += $(obj)/early_sha1.o
> +vmlinux-objs-$(CONFIG_SECURE_LAUNCH_SHA256) += $(obj)/early_sha256.o
> +vmlinux-objs-$(CONFIG_SECURE_LAUNCH_SHA512) += $(obj)/early_sha512.o
> +
>  # The compressed kernel is built with -fPIC/-fPIE so that a boot loader
>  # can place it anywhere in memory and it will still run. However, since
>  # it is executed as-is without any ELF relocation processing performed
> diff --git a/arch/x86/boot/compressed/early_sha1.c 
> b/arch/x86/boot/compressed/early_sha1.c
> new file mode 100644
> index 000..198c46d
> --- /dev/null
> +++ b/arch/x86/boot/compressed/early_sha1.c
> @@ -0,0 +1,104 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2020, Oracle and/or its affiliates.
> + * Copyright (c) 2020 Apertus Solutions, LLC.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "early_sha1.h"
> +
> +#define SHA1_DISABLE_EXPORT
> +#include "../../../../lib/sha1.c"
> +
> +/* The SHA1 implementation in lib/sha1.c was written to get the workspace
> + * buffer as a parameter. This wrapper function provides a container
> + * around a temporary workspace that is cleared after the transform 
> completes.
> + */
> +static void __sha_transform(u32 *digest, const char *data)
> +{
> +   u32 ws[SHA1_WORKSPACE_WORDS];
> +
> +   sha1_transform(digest, data, ws);
> +
> +   memset(ws, 0, sizeof(ws));
> +   /*
> +* As this is cryptographic code, prevent the memset 0 from being
> +* optimized out potentially leaving secrets in memory.
> +*/
> +   wmb();

You can use memzero_explicit instead of open coding it.

Regards,
Jason

[tip: x86/cleanups] x86/idt: Make idt_descr static

[tip-bot2 for Jason Andryuk]

The following commit has been merged into the x86/cleanups branch of tip:

Commit-ID: 286d966b21587b6303081b902f5c5e30b691baf5
Gitweb:
https://git.kernel.org/tip/286d966b21587b6303081b902f5c5e30b691baf5
Author:Jason Andryuk 
AuthorDate:Fri, 19 Jun 2020 16:51:02 -04:00
Committer: Borislav Petkov 
CommitterDate: Sat, 20 Jun 2020 11:47:35 +02:00

x86/idt: Make idt_descr static

Commit

  3e77abda65b1 ("x86/idt: Consolidate idt functionality")

states that idt_descr could be made static, but it did not actually make
the change. Make it static now.

Fixes: 3e77abda65b1 ("x86/idt: Consolidate idt functionality")
Signed-off-by: Jason Andryuk 
Signed-off-by: Borislav Petkov 
Link: https://lkml.kernel.org/r/20200619205103.30873-1-jandr...@gmail.com
---
 arch/x86/kernel/idt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/idt.c b/arch/x86/kernel/idt.c
index 0db2120..7ecf9ba 100644
--- a/arch/x86/kernel/idt.c
+++ b/arch/x86/kernel/idt.c
@@ -160,7 +160,7 @@ static const __initconst struct idt_data apic_idts[] = {
 /* Must be page-aligned because the real IDT is used in the cpu entry area */
 static gate_desc idt_table[IDT_ENTRIES] __page_aligned_bss;
 
-struct desc_ptr idt_descr __ro_after_init = {
+static struct desc_ptr idt_descr __ro_after_init = {
.size   = IDT_TABLE_SIZE - 1,
.address= (unsigned long) idt_table,
 };

[PATCH] x86/idt: Make idt_descr static

[Jason Andryuk]

Commit 3e77abda65b1 ("x86/idt: Consolidate idt functionality")'s commit
message stated idt_descr could be made static, but it did not actually
make the change.  Make it static now.

Fixes: 3e77abda65b1 ("x86/idt: Consolidate idt functionality")
Signed-off-by: Jason Andryuk 
---
 arch/x86/kernel/idt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/idt.c b/arch/x86/kernel/idt.c
index 0db21206f2f3..7ecf9babf0cb 100644
--- a/arch/x86/kernel/idt.c
+++ b/arch/x86/kernel/idt.c
@@ -160,7 +160,7 @@ static const __initconst struct idt_data apic_idts[] = {
 /* Must be page-aligned because the real IDT is used in the cpu entry area */
 static gate_desc idt_table[IDT_ENTRIES] __page_aligned_bss;
 
-struct desc_ptr idt_descr __ro_after_init = {
+static struct desc_ptr idt_descr __ro_after_init = {
.size   = IDT_TABLE_SIZE - 1,
.address= (unsigned long) idt_table,
 };
-- 
2.25.1

[PATCH 4.14] x86/pae: use 64 bit atomic xchg function in native_ptep_get_and_clear

[Jason Andryuk]

From: Juergen Gross 

commit b2d7a075a1ccef2fb321d595802190c8e9b39004 upstream

Using only 32-bit writes for the pte will result in an intermediate
L1TF vulnerable PTE. When running as a Xen PV guest this will at once
switch the guest to shadow mode resulting in a loss of performance.

Use arch_atomic64_xchg() instead which will perform the requested
operation atomically with all 64 bits.

Some performance considerations according to:

https://software.intel.com/sites/default/files/managed/ad/dc/Intel-Xeon-Scalable-Processor-throughput-latency.pdf

The main number should be the latency, as there is no tight loop around
native_ptep_get_and_clear().

"lock cmpxchg8b" has a latency of 20 cycles, while "lock xchg" (with a
memory operand) isn't mentioned in that document. "lock xadd" (with xadd
having 3 cycles less latency than xchg) has a latency of 11, so we can
assume a latency of 14 for "lock xchg".

Signed-off-by: Juergen Gross 
Reviewed-by: Thomas Gleixner 
Reviewed-by: Jan Beulich 
Tested-by: Jason Andryuk 
Signed-off-by: Boris Ostrovsky 
Atomic operations gained an arch_ prefix in commit
8bf705d130396e69c04cd8e6e010244ad2ce71f4
s/arch_atomic64_xchg/atomic64_xchg/ for backport.
Signed-off-by: Jason Andryuk 
---
 arch/x86/include/asm/pgtable-3level.h | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/pgtable-3level.h 
b/arch/x86/include/asm/pgtable-3level.h
index 9dc19b4a2a87..c5d4931d1ef9 100644
--- a/arch/x86/include/asm/pgtable-3level.h
+++ b/arch/x86/include/asm/pgtable-3level.h
@@ -2,6 +2,8 @@
 #ifndef _ASM_X86_PGTABLE_3LEVEL_H
 #define _ASM_X86_PGTABLE_3LEVEL_H
 
+#include 
+
 /*
  * Intel Physical Address Extension (PAE) Mode - three-level page
  * tables on PPro+ CPUs.
@@ -147,10 +149,7 @@ static inline pte_t native_ptep_get_and_clear(pte_t *ptep)
 {
pte_t res;
 
-   /* xchg acts as a barrier before the setting of the high bits */
-   res.pte_low = xchg(>pte_low, 0);
-   res.pte_high = ptep->pte_high;
-   ptep->pte_high = 0;
+   res.pte = (pteval_t)atomic64_xchg((atomic64_t *)ptep, 0);
 
return res;
 }
-- 
2.17.1

[PATCH 4.14] x86/pae: use 64 bit atomic xchg function in native_ptep_get_and_clear

[Jason Andryuk]

From: Juergen Gross 

commit b2d7a075a1ccef2fb321d595802190c8e9b39004 upstream

Using only 32-bit writes for the pte will result in an intermediate
L1TF vulnerable PTE. When running as a Xen PV guest this will at once
switch the guest to shadow mode resulting in a loss of performance.

Use arch_atomic64_xchg() instead which will perform the requested
operation atomically with all 64 bits.

Some performance considerations according to:

https://software.intel.com/sites/default/files/managed/ad/dc/Intel-Xeon-Scalable-Processor-throughput-latency.pdf

The main number should be the latency, as there is no tight loop around
native_ptep_get_and_clear().

"lock cmpxchg8b" has a latency of 20 cycles, while "lock xchg" (with a
memory operand) isn't mentioned in that document. "lock xadd" (with xadd
having 3 cycles less latency than xchg) has a latency of 11, so we can
assume a latency of 14 for "lock xchg".

Signed-off-by: Juergen Gross 
Reviewed-by: Thomas Gleixner 
Reviewed-by: Jan Beulich 
Tested-by: Jason Andryuk 
Signed-off-by: Boris Ostrovsky 
Atomic operations gained an arch_ prefix in commit
8bf705d130396e69c04cd8e6e010244ad2ce71f4
s/arch_atomic64_xchg/atomic64_xchg/ for backport.
Signed-off-by: Jason Andryuk 
---
 arch/x86/include/asm/pgtable-3level.h | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/pgtable-3level.h 
b/arch/x86/include/asm/pgtable-3level.h
index 9dc19b4a2a87..c5d4931d1ef9 100644
--- a/arch/x86/include/asm/pgtable-3level.h
+++ b/arch/x86/include/asm/pgtable-3level.h
@@ -2,6 +2,8 @@
 #ifndef _ASM_X86_PGTABLE_3LEVEL_H
 #define _ASM_X86_PGTABLE_3LEVEL_H
 
+#include 
+
 /*
  * Intel Physical Address Extension (PAE) Mode - three-level page
  * tables on PPro+ CPUs.
@@ -147,10 +149,7 @@ static inline pte_t native_ptep_get_and_clear(pte_t *ptep)
 {
pte_t res;
 
-   /* xchg acts as a barrier before the setting of the high bits */
-   res.pte_low = xchg(>pte_low, 0);
-   res.pte_high = ptep->pte_high;
-   ptep->pte_high = 0;
+   res.pte = (pteval_t)atomic64_xchg((atomic64_t *)ptep, 0);
 
return res;
 }
-- 
2.17.1

Re: [PATCH 4.14 102/115] x86/pae: use 64 bit atomic xchg function in native_ptep_get_and_clear

[Jason Andryuk]

On Thu, Sep 13, 2018 at 9:48 AM Greg Kroah-Hartman
 wrote:
>
> 4.14-stable review patch.  If anyone has any objections, please let me know.
>
> --
>
> From: Juergen Gross 
>
> commit b2d7a075a1ccef2fb321d595802190c8e9b39004 upstream.
>
> Using only 32-bit writes for the pte will result in an intermediate
> L1TF vulnerable PTE. When running as a Xen PV guest this will at once
> switch the guest to shadow mode resulting in a loss of performance.
>
> Use arch_atomic64_xchg() instead which will perform the requested
> operation atomically with all 64 bits.
>
> Some performance considerations according to:
>
> https://software.intel.com/sites/default/files/managed/ad/dc/Intel-Xeon-Scalable-Processor-throughput-latency.pdf
>
> The main number should be the latency, as there is no tight loop around
> native_ptep_get_and_clear().
>
> "lock cmpxchg8b" has a latency of 20 cycles, while "lock xchg" (with a
> memory operand) isn't mentioned in that document. "lock xadd" (with xadd
> having 3 cycles less latency than xchg) has a latency of 11, so we can
> assume a latency of 14 for "lock xchg".
>
> Signed-off-by: Juergen Gross 
> Reviewed-by: Thomas Gleixner 
> Reviewed-by: Jan Beulich 
> Tested-by: Jason Andryuk 
> Signed-off-by: Boris Ostrovsky 
> Signed-off-by: Greg Kroah-Hartman 
>
> ---
>  arch/x86/include/asm/pgtable-3level.h |7 +++
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> --- a/arch/x86/include/asm/pgtable-3level.h
> +++ b/arch/x86/include/asm/pgtable-3level.h
> @@ -2,6 +2,8 @@
>  #ifndef _ASM_X86_PGTABLE_3LEVEL_H
>  #define _ASM_X86_PGTABLE_3LEVEL_H
>
> +#include 
> +
>  /*
>   * Intel Physical Address Extension (PAE) Mode - three-level page
>   * tables on PPro+ CPUs.
> @@ -147,10 +149,7 @@ static inline pte_t native_ptep_get_and_
>  {
> pte_t res;
>
> -   /* xchg acts as a barrier before the setting of the high bits */
> -   res.pte_low = xchg(>pte_low, 0);
> -   res.pte_high = ptep->pte_high;
> -   ptep->pte_high = 0;
> +   res.pte = (pteval_t)arch_atomic64_xchg((atomic64_t *)ptep, 0);

For 4.14, I had to change this to atomic64_xchg since
arch_atomic64_xchg doesn't exist.

kernel-source/arch/x86/include/asm/pgtable-3level.h:152:22: error:
implicit declaration of function 'arch_atomic64_xchg'
[-Werror=implicit-function-declaration]

The same is probably needed for earlier versions as well.

Regards,
Jason

>
> return res;
>  }
>
>

Re: [PATCH 4.14 102/115] x86/pae: use 64 bit atomic xchg function in native_ptep_get_and_clear

[Jason Andryuk]

On Thu, Sep 13, 2018 at 9:48 AM Greg Kroah-Hartman
 wrote:
>
> 4.14-stable review patch.  If anyone has any objections, please let me know.
>
> --
>
> From: Juergen Gross 
>
> commit b2d7a075a1ccef2fb321d595802190c8e9b39004 upstream.
>
> Using only 32-bit writes for the pte will result in an intermediate
> L1TF vulnerable PTE. When running as a Xen PV guest this will at once
> switch the guest to shadow mode resulting in a loss of performance.
>
> Use arch_atomic64_xchg() instead which will perform the requested
> operation atomically with all 64 bits.
>
> Some performance considerations according to:
>
> https://software.intel.com/sites/default/files/managed/ad/dc/Intel-Xeon-Scalable-Processor-throughput-latency.pdf
>
> The main number should be the latency, as there is no tight loop around
> native_ptep_get_and_clear().
>
> "lock cmpxchg8b" has a latency of 20 cycles, while "lock xchg" (with a
> memory operand) isn't mentioned in that document. "lock xadd" (with xadd
> having 3 cycles less latency than xchg) has a latency of 11, so we can
> assume a latency of 14 for "lock xchg".
>
> Signed-off-by: Juergen Gross 
> Reviewed-by: Thomas Gleixner 
> Reviewed-by: Jan Beulich 
> Tested-by: Jason Andryuk 
> Signed-off-by: Boris Ostrovsky 
> Signed-off-by: Greg Kroah-Hartman 
>
> ---
>  arch/x86/include/asm/pgtable-3level.h |7 +++
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> --- a/arch/x86/include/asm/pgtable-3level.h
> +++ b/arch/x86/include/asm/pgtable-3level.h
> @@ -2,6 +2,8 @@
>  #ifndef _ASM_X86_PGTABLE_3LEVEL_H
>  #define _ASM_X86_PGTABLE_3LEVEL_H
>
> +#include 
> +
>  /*
>   * Intel Physical Address Extension (PAE) Mode - three-level page
>   * tables on PPro+ CPUs.
> @@ -147,10 +149,7 @@ static inline pte_t native_ptep_get_and_
>  {
> pte_t res;
>
> -   /* xchg acts as a barrier before the setting of the high bits */
> -   res.pte_low = xchg(>pte_low, 0);
> -   res.pte_high = ptep->pte_high;
> -   ptep->pte_high = 0;
> +   res.pte = (pteval_t)arch_atomic64_xchg((atomic64_t *)ptep, 0);

For 4.14, I had to change this to atomic64_xchg since
arch_atomic64_xchg doesn't exist.

kernel-source/arch/x86/include/asm/pgtable-3level.h:152:22: error:
implicit declaration of function 'arch_atomic64_xchg'
[-Werror=implicit-function-declaration]

The same is probably needed for earlier versions as well.

Regards,
Jason

>
> return res;
>  }
>
>

Re: [PATCH] i2c-hid: Fix "incomplete report" noise

[Jason Andryuk]

Ping?

The logging here is very excessive.  If not this change, then some
other change is needed to cut down on the sheer quantity of messages.

Thanks,
Jason


On Fri, Jun 22, 2018 at 12:25 PM, Jason Andryuk  wrote:
> Commit ac75a041048b ("HID: i2c-hid: fix size check and type usage")
> started writing messages when the ret_size is <= 2 from i2c_master_recv.
> However, my device i2c-DLL07D1 returns 2 for a short period of time
> (~0.5s) after I stop moving the pointing stick or touchpad.  It varies,
> but you get ~50 messages each time which spams the log hard.
> [  95.925055] i2c_hid i2c-DLL07D1:01: i2c_hid_get_input: incomplete report 
> (83/2)
>
> This has also been observed with a i2c-ALP0017.
> [ 1781.266353] i2c_hid i2c-ALP0017:00: i2c_hid_get_input: incomplete report 
> (30/2)
>
> Only print the message when ret_size is totally invalid and less than 2
> to cut down on the log spam.
>
> Reported-by: John Smith 
> Cc: sta...@vger.kernel.org
> Signed-off-by: Jason Andryuk 
> ---
> John Smith originally reported this, but his post did not include a git
> formatted patch nor a Signed-off-by.
> https://www.spinics.net/lists/linux-input/msg56171.html
> https://patchwork.kernel.org/patch/10374383/
>
> When ret_size is 2, hid_input_report is passed 0 and returns early.  ret_size
> == 2 seems to be a header size saying there is no content.  Should
> i2c_hid_get_input just return early in that case?
>
> Also, should this condition be noted to stop an interrupt from firing to
> avoid the ~50 bogus messages?
>
>  drivers/hid/i2c-hid/i2c-hid.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
> index c1652bb7bd15..eae0cb3ddec6 100644
> --- a/drivers/hid/i2c-hid/i2c-hid.c
> +++ b/drivers/hid/i2c-hid/i2c-hid.c
> @@ -484,7 +484,7 @@ static void i2c_hid_get_input(struct i2c_hid *ihid)
> return;
> }
>
> -   if ((ret_size > size) || (ret_size <= 2)) {
> +   if ((ret_size > size) || (ret_size < 2)) {
> dev_err(>client->dev, "%s: incomplete report (%d/%d)\n",
> __func__, size, ret_size);
> return;
> --
> 2.17.1
>

Re: [PATCH] i2c-hid: Fix "incomplete report" noise

[Jason Andryuk]

Ping?

The logging here is very excessive.  If not this change, then some
other change is needed to cut down on the sheer quantity of messages.

Thanks,
Jason


On Fri, Jun 22, 2018 at 12:25 PM, Jason Andryuk  wrote:
> Commit ac75a041048b ("HID: i2c-hid: fix size check and type usage")
> started writing messages when the ret_size is <= 2 from i2c_master_recv.
> However, my device i2c-DLL07D1 returns 2 for a short period of time
> (~0.5s) after I stop moving the pointing stick or touchpad.  It varies,
> but you get ~50 messages each time which spams the log hard.
> [  95.925055] i2c_hid i2c-DLL07D1:01: i2c_hid_get_input: incomplete report 
> (83/2)
>
> This has also been observed with a i2c-ALP0017.
> [ 1781.266353] i2c_hid i2c-ALP0017:00: i2c_hid_get_input: incomplete report 
> (30/2)
>
> Only print the message when ret_size is totally invalid and less than 2
> to cut down on the log spam.
>
> Reported-by: John Smith 
> Cc: sta...@vger.kernel.org
> Signed-off-by: Jason Andryuk 
> ---
> John Smith originally reported this, but his post did not include a git
> formatted patch nor a Signed-off-by.
> https://www.spinics.net/lists/linux-input/msg56171.html
> https://patchwork.kernel.org/patch/10374383/
>
> When ret_size is 2, hid_input_report is passed 0 and returns early.  ret_size
> == 2 seems to be a header size saying there is no content.  Should
> i2c_hid_get_input just return early in that case?
>
> Also, should this condition be noted to stop an interrupt from firing to
> avoid the ~50 bogus messages?
>
>  drivers/hid/i2c-hid/i2c-hid.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
> index c1652bb7bd15..eae0cb3ddec6 100644
> --- a/drivers/hid/i2c-hid/i2c-hid.c
> +++ b/drivers/hid/i2c-hid/i2c-hid.c
> @@ -484,7 +484,7 @@ static void i2c_hid_get_input(struct i2c_hid *ihid)
> return;
> }
>
> -   if ((ret_size > size) || (ret_size <= 2)) {
> +   if ((ret_size > size) || (ret_size < 2)) {
> dev_err(>client->dev, "%s: incomplete report (%d/%d)\n",
> __func__, size, ret_size);
> return;
> --
> 2.17.1
>

[PATCH] i2c-hid: Fix "incomplete report" noise

[Jason Andryuk]

Commit ac75a041048b ("HID: i2c-hid: fix size check and type usage")
started writing messages when the ret_size is <= 2 from i2c_master_recv.
However, my device i2c-DLL07D1 returns 2 for a short period of time
(~0.5s) after I stop moving the pointing stick or touchpad.  It varies,
but you get ~50 messages each time which spams the log hard.
[  95.925055] i2c_hid i2c-DLL07D1:01: i2c_hid_get_input: incomplete report 
(83/2)

This has also been observed with a i2c-ALP0017.
[ 1781.266353] i2c_hid i2c-ALP0017:00: i2c_hid_get_input: incomplete report 
(30/2)

Only print the message when ret_size is totally invalid and less than 2
to cut down on the log spam.

Reported-by: John Smith 
Cc: sta...@vger.kernel.org
Signed-off-by: Jason Andryuk 
---
John Smith originally reported this, but his post did not include a git
formatted patch nor a Signed-off-by.
https://www.spinics.net/lists/linux-input/msg56171.html
https://patchwork.kernel.org/patch/10374383/

When ret_size is 2, hid_input_report is passed 0 and returns early.  ret_size
== 2 seems to be a header size saying there is no content.  Should
i2c_hid_get_input just return early in that case?

Also, should this condition be noted to stop an interrupt from firing to
avoid the ~50 bogus messages?

 drivers/hid/i2c-hid/i2c-hid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index c1652bb7bd15..eae0cb3ddec6 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -484,7 +484,7 @@ static void i2c_hid_get_input(struct i2c_hid *ihid)
return;
}
 
-   if ((ret_size > size) || (ret_size <= 2)) {
+   if ((ret_size > size) || (ret_size < 2)) {
dev_err(>client->dev, "%s: incomplete report (%d/%d)\n",
__func__, size, ret_size);
return;
-- 
2.17.1

[PATCH] i2c-hid: Fix "incomplete report" noise

[Jason Andryuk]

Commit ac75a041048b ("HID: i2c-hid: fix size check and type usage")
started writing messages when the ret_size is <= 2 from i2c_master_recv.
However, my device i2c-DLL07D1 returns 2 for a short period of time
(~0.5s) after I stop moving the pointing stick or touchpad.  It varies,
but you get ~50 messages each time which spams the log hard.
[  95.925055] i2c_hid i2c-DLL07D1:01: i2c_hid_get_input: incomplete report 
(83/2)

This has also been observed with a i2c-ALP0017.
[ 1781.266353] i2c_hid i2c-ALP0017:00: i2c_hid_get_input: incomplete report 
(30/2)

Only print the message when ret_size is totally invalid and less than 2
to cut down on the log spam.

Reported-by: John Smith 
Cc: sta...@vger.kernel.org
Signed-off-by: Jason Andryuk 
---
John Smith originally reported this, but his post did not include a git
formatted patch nor a Signed-off-by.
https://www.spinics.net/lists/linux-input/msg56171.html
https://patchwork.kernel.org/patch/10374383/

When ret_size is 2, hid_input_report is passed 0 and returns early.  ret_size
== 2 seems to be a header size saying there is no content.  Should
i2c_hid_get_input just return early in that case?

Also, should this condition be noted to stop an interrupt from firing to
avoid the ~50 bogus messages?

 drivers/hid/i2c-hid/i2c-hid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index c1652bb7bd15..eae0cb3ddec6 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -484,7 +484,7 @@ static void i2c_hid_get_input(struct i2c_hid *ihid)
return;
}
 
-   if ((ret_size > size) || (ret_size <= 2)) {
+   if ((ret_size > size) || (ret_size < 2)) {
dev_err(>client->dev, "%s: incomplete report (%d/%d)\n",
__func__, size, ret_size);
return;
-- 
2.17.1

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

[Jason Andryuk]

Hi, Oleksandr.

On Thu, Apr 19, 2018 at 9:39 AM, Oleksandr Andrushchenko
 wrote:



> @@ -241,60 +242,84 @@ static int xenkbd_probe(struct xenbus_device *dev,
> }
>
> /* keyboard */
> -   kbd = input_allocate_device();
> -   if (!kbd)
> -   goto error_nomem;
> -   kbd->name = "Xen Virtual Keyboard";
> -   kbd->phys = info->phys;
> -   kbd->id.bustype = BUS_PCI;
> -   kbd->id.vendor = 0x5853;
> -   kbd->id.product = 0x;
> -
> -   __set_bit(EV_KEY, kbd->evbit);
> -   for (i = KEY_ESC; i < KEY_UNKNOWN; i++)
> -   __set_bit(i, kbd->keybit);
> -   for (i = KEY_OK; i < KEY_MAX; i++)
> -   __set_bit(i, kbd->keybit);
> -
> -   ret = input_register_device(kbd);
> -   if (ret) {
> -   input_free_device(kbd);
> -   xenbus_dev_fatal(dev, ret, "input_register_device(kbd)");
> -   goto error;
> +   if (!no_kbd_dev) {

My earlier suggestion on the option name was aimed at replacing the above with:
   if (kbd_dev) {

I find it easier to read then the double negative !no_kbd_dev.  But
it's only used once, so it's not a big deal either way.



>
> -   __set_bit(EV_KEY, ptr->evbit);
> -   for (i = BTN_LEFT; i <= BTN_TASK; i++)
> -   __set_bit(i, ptr->keybit);
> +   ptr = input_allocate_device();
> +   if (!ptr)
> +   goto error_nomem;
> +   ptr->name = "Xen Virtual Pointer";
> +   ptr->phys = info->phys;
> +   ptr->id.bustype = BUS_PCI;
> +   ptr->id.vendor = 0x5853;
> +   ptr->id.product = 0xfffe;
> +
> +   if (abs) {
> +   __set_bit(EV_ABS, ptr->evbit);
> +   input_set_abs_params(ptr, ABS_X, 0, 
> ptr_size[KPARAM_X], 0, 0);
> +   input_set_abs_params(ptr, ABS_Y, 0, 
> ptr_size[KPARAM_Y], 0, 0);

Just noticed these lines now exceed 80 columns.

Otherwise it's just code motion and fine by me.

Regards,
Jason

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

[Jason Andryuk]

Hi, Oleksandr.

On Thu, Apr 19, 2018 at 9:39 AM, Oleksandr Andrushchenko
 wrote:



> @@ -241,60 +242,84 @@ static int xenkbd_probe(struct xenbus_device *dev,
> }
>
> /* keyboard */
> -   kbd = input_allocate_device();
> -   if (!kbd)
> -   goto error_nomem;
> -   kbd->name = "Xen Virtual Keyboard";
> -   kbd->phys = info->phys;
> -   kbd->id.bustype = BUS_PCI;
> -   kbd->id.vendor = 0x5853;
> -   kbd->id.product = 0x;
> -
> -   __set_bit(EV_KEY, kbd->evbit);
> -   for (i = KEY_ESC; i < KEY_UNKNOWN; i++)
> -   __set_bit(i, kbd->keybit);
> -   for (i = KEY_OK; i < KEY_MAX; i++)
> -   __set_bit(i, kbd->keybit);
> -
> -   ret = input_register_device(kbd);
> -   if (ret) {
> -   input_free_device(kbd);
> -   xenbus_dev_fatal(dev, ret, "input_register_device(kbd)");
> -   goto error;
> +   if (!no_kbd_dev) {

My earlier suggestion on the option name was aimed at replacing the above with:
   if (kbd_dev) {

I find it easier to read then the double negative !no_kbd_dev.  But
it's only used once, so it's not a big deal either way.



>
> -   __set_bit(EV_KEY, ptr->evbit);
> -   for (i = BTN_LEFT; i <= BTN_TASK; i++)
> -   __set_bit(i, ptr->keybit);
> +   ptr = input_allocate_device();
> +   if (!ptr)
> +   goto error_nomem;
> +   ptr->name = "Xen Virtual Pointer";
> +   ptr->phys = info->phys;
> +   ptr->id.bustype = BUS_PCI;
> +   ptr->id.vendor = 0x5853;
> +   ptr->id.product = 0xfffe;
> +
> +   if (abs) {
> +   __set_bit(EV_ABS, ptr->evbit);
> +   input_set_abs_params(ptr, ABS_X, 0, 
> ptr_size[KPARAM_X], 0, 0);
> +   input_set_abs_params(ptr, ABS_Y, 0, 
> ptr_size[KPARAM_Y], 0, 0);

Just noticed these lines now exceed 80 columns.

Otherwise it's just code motion and fine by me.

Regards,
Jason

Re: [Xen-devel] [PATCH] xen-netfront: Fix hang on device removal

[Jason Andryuk]

On Thu, Apr 19, 2018 at 4:09 PM, Simon Gaiser
<si...@invisiblethingslab.com> wrote:
> Jason Andryuk:
>> On Thu, Apr 19, 2018 at 2:10 PM, Simon Gaiser
>> <si...@invisiblethingslab.com> wrote:
>>> Jason Andryuk:
>>>> A toolstack may delete the vif frontend and backend xenstore entries
>>>> while xen-netfront is in the removal code path.  In that case, the
>>>> checks for xenbus_read_driver_state would return XenbusStateUnknown, and
>>>> xennet_remove would hang indefinitely.  This hang prevents system
>>>> shutdown.
>>>>
>>>> xennet_remove must be able to handle XenbusStateUnknown, and
>>>> netback_changed must also wake up the wake_queue for that state as well.
>>>>
>>>> Fixes: 5b5971df3bc2 ("xen-netfront: remove warning when unloading module")
>>>
>>> I think this should go into stable since AFAIK the hanging network
>>> device can only be fixed by rebooting the guest. AFAICS this affects all
>>> 4.* branches since 5b5971df3bc2 got backported to them.
>>>
>>> Upstream commit c2d2e6738a209f0f9dffa2dc8e7292fc45360d61.
>>
>> Simon,
>>
>> Yes, I agree.  I actually submitted the request to stable earlier
>> today, so hopefully it gets added soon.
>
> Ok, great. (I checked the stable patch queue, but didn't check the
> mailing list archive).
>
>> Have you experienced this hang?
>
> Yes, it's affecting the kernel shipped by Qubes OS (see [1]).

Ok, interesting.  I tracked down this bug with older xenvm tools, and
I didn't know if libxl tools were also affected.

Greg KH added the patch to the stable queue, so it's in the process.

Regards,
Jason

Re: [Xen-devel] [PATCH] xen-netfront: Fix hang on device removal

[Jason Andryuk]

On Thu, Apr 19, 2018 at 4:09 PM, Simon Gaiser
 wrote:
> Jason Andryuk:
>> On Thu, Apr 19, 2018 at 2:10 PM, Simon Gaiser
>>  wrote:
>>> Jason Andryuk:
>>>> A toolstack may delete the vif frontend and backend xenstore entries
>>>> while xen-netfront is in the removal code path.  In that case, the
>>>> checks for xenbus_read_driver_state would return XenbusStateUnknown, and
>>>> xennet_remove would hang indefinitely.  This hang prevents system
>>>> shutdown.
>>>>
>>>> xennet_remove must be able to handle XenbusStateUnknown, and
>>>> netback_changed must also wake up the wake_queue for that state as well.
>>>>
>>>> Fixes: 5b5971df3bc2 ("xen-netfront: remove warning when unloading module")
>>>
>>> I think this should go into stable since AFAIK the hanging network
>>> device can only be fixed by rebooting the guest. AFAICS this affects all
>>> 4.* branches since 5b5971df3bc2 got backported to them.
>>>
>>> Upstream commit c2d2e6738a209f0f9dffa2dc8e7292fc45360d61.
>>
>> Simon,
>>
>> Yes, I agree.  I actually submitted the request to stable earlier
>> today, so hopefully it gets added soon.
>
> Ok, great. (I checked the stable patch queue, but didn't check the
> mailing list archive).
>
>> Have you experienced this hang?
>
> Yes, it's affecting the kernel shipped by Qubes OS (see [1]).

Ok, interesting.  I tracked down this bug with older xenvm tools, and
I didn't know if libxl tools were also affected.

Greg KH added the patch to the stable queue, so it's in the process.

Regards,
Jason

Re: [Xen-devel] [PATCH] xen-netfront: Fix hang on device removal

[Jason Andryuk]

On Thu, Apr 19, 2018 at 2:10 PM, Simon Gaiser
<si...@invisiblethingslab.com> wrote:
> Jason Andryuk:
>> A toolstack may delete the vif frontend and backend xenstore entries
>> while xen-netfront is in the removal code path.  In that case, the
>> checks for xenbus_read_driver_state would return XenbusStateUnknown, and
>> xennet_remove would hang indefinitely.  This hang prevents system
>> shutdown.
>>
>> xennet_remove must be able to handle XenbusStateUnknown, and
>> netback_changed must also wake up the wake_queue for that state as well.
>>
>> Fixes: 5b5971df3bc2 ("xen-netfront: remove warning when unloading module")
>
> I think this should go into stable since AFAIK the hanging network
> device can only be fixed by rebooting the guest. AFAICS this affects all
> 4.* branches since 5b5971df3bc2 got backported to them.
>
> Upstream commit c2d2e6738a209f0f9dffa2dc8e7292fc45360d61.

Simon,

Yes, I agree.  I actually submitted the request to stable earlier
today, so hopefully it gets added soon.

Have you experienced this hang?

Regards,
Jason

Re: [Xen-devel] [PATCH] xen-netfront: Fix hang on device removal

[Jason Andryuk]

On Thu, Apr 19, 2018 at 2:10 PM, Simon Gaiser
 wrote:
> Jason Andryuk:
>> A toolstack may delete the vif frontend and backend xenstore entries
>> while xen-netfront is in the removal code path.  In that case, the
>> checks for xenbus_read_driver_state would return XenbusStateUnknown, and
>> xennet_remove would hang indefinitely.  This hang prevents system
>> shutdown.
>>
>> xennet_remove must be able to handle XenbusStateUnknown, and
>> netback_changed must also wake up the wake_queue for that state as well.
>>
>> Fixes: 5b5971df3bc2 ("xen-netfront: remove warning when unloading module")
>
> I think this should go into stable since AFAIK the hanging network
> device can only be fixed by rebooting the guest. AFAICS this affects all
> 4.* branches since 5b5971df3bc2 got backported to them.
>
> Upstream commit c2d2e6738a209f0f9dffa2dc8e7292fc45360d61.

Simon,

Yes, I agree.  I actually submitted the request to stable earlier
today, so hopefully it gets added soon.

Have you experienced this hang?

Regards,
Jason

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

[Jason Andryuk]

On Thu, Apr 19, 2018 at 9:01 AM, Oleksandr Andrushchenko
 wrote:
>
> Ok, so I'll send v2 with the following changes:
>
> diff --git a/drivers/input/misc/xen-kbdfront.c
> b/drivers/input/misc/xen-kbdfront.c
> index a3306aad40b0..d8cca212f737 100644
> --- a/drivers/input/misc/xen-kbdfront.c
> +++ b/drivers/input/misc/xen-kbdfront.c
> @@ -51,13 +51,13 @@ module_param_array(ptr_size, int, NULL, 0444);
>  MODULE_PARM_DESC(ptr_size,
> "Pointing device width, height in pixels (default 800,600)");
>
> -static unsigned int no_ptr_dev;
> -module_param(no_ptr_dev, uint, 0);
> +static bool no_ptr_dev;
> +module_param(no_ptr_dev, bool, 0);
>  MODULE_PARM_DESC(no_ptr_dev,
> "If set then no virtual pointing device exposed to the guest");
>
> -static unsigned int no_kbd_dev;
> -module_param(no_kbd_dev, uint, 0);
> +static bool no_kbd_dev;
> +module_param(no_kbd_dev, bool, 0);
>  MODULE_PARM_DESC(no_kbd_dev,
> "If set then no virtual keyboard device exposed to the guest");

I prefer direct logic over inverse logic.  Maybe just use kbd_dev,
default to true, but allow it to be set off?

static bool kbd_dev = true;
module_param(kbd_dev, bool, 0);

Regards,
Jason

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

[Jason Andryuk]

On Thu, Apr 19, 2018 at 9:01 AM, Oleksandr Andrushchenko
 wrote:
>
> Ok, so I'll send v2 with the following changes:
>
> diff --git a/drivers/input/misc/xen-kbdfront.c
> b/drivers/input/misc/xen-kbdfront.c
> index a3306aad40b0..d8cca212f737 100644
> --- a/drivers/input/misc/xen-kbdfront.c
> +++ b/drivers/input/misc/xen-kbdfront.c
> @@ -51,13 +51,13 @@ module_param_array(ptr_size, int, NULL, 0444);
>  MODULE_PARM_DESC(ptr_size,
> "Pointing device width, height in pixels (default 800,600)");
>
> -static unsigned int no_ptr_dev;
> -module_param(no_ptr_dev, uint, 0);
> +static bool no_ptr_dev;
> +module_param(no_ptr_dev, bool, 0);
>  MODULE_PARM_DESC(no_ptr_dev,
> "If set then no virtual pointing device exposed to the guest");
>
> -static unsigned int no_kbd_dev;
> -module_param(no_kbd_dev, uint, 0);
> +static bool no_kbd_dev;
> +module_param(no_kbd_dev, bool, 0);
>  MODULE_PARM_DESC(no_kbd_dev,
> "If set then no virtual keyboard device exposed to the guest");

I prefer direct logic over inverse logic.  Maybe just use kbd_dev,
default to true, but allow it to be set off?

static bool kbd_dev = true;
module_param(kbd_dev, bool, 0);

Regards,
Jason

[PATCH v3] i2c: i801: blacklist Host Notify on HP EliteBook G3 850

[Jason Andryuk]

The HP EliteBook G3 850 has a weird bug where a subsequent cold boot
hangs while plugged in if Linux enables the Host Notify features of
i2c-i801.  The cold boot hang depends on how the system boots.  It does
not hang on UEFI Grub text boot or legacy Grub text boot.  But it does
hang on legacy Grub graphical boot and Intel Boot Agent PXE text boot.
Booting unplugged is not affected.

Disabling the Host Notify feature with disable_feature=0x20 works around
the bug, so automatically do so based on DMI information.

More information can be found here:
https://www.spinics.net/lists/linux-i2c/msg33938.html

Signed-off-by: Jason Andryuk <jandr...@gmail.com>
Reviewed-by: Jean Delvare <jdelv...@suse.de>
Cc: sta...@vger.kernel.org
---
v3: Switch to DMI_EXACT_MATCH and add empty element to array

 drivers/i2c/busses/i2c-i801.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 692b34125866..11149ddae745 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1042,6 +1042,27 @@ static const struct pci_device_id i801_ids[] = {
 MODULE_DEVICE_TABLE(pci, i801_ids);
 
 #if defined CONFIG_X86 && defined CONFIG_DMI
+static const struct dmi_system_id host_notify_dmi_blacklist[] = {
+   {
+   .ident = "HP EliteBook G3 850",
+   .matches = {
+   DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "HP"),
+   DMI_EXACT_MATCH(DMI_PRODUCT_NAME,
+   "HP EliteBook 850 G3"),
+   },
+   },
+   {}
+};
+
+static void blacklist_features(struct i801_priv *priv)
+{
+   if (dmi_check_system(host_notify_dmi_blacklist)) {
+   dev_warn(>pci_dev->dev,
+"SMBus Host Notify disabled on this system");
+   priv->features &= ~FEATURE_HOST_NOTIFY;
+   }
+}
+
 static unsigned char apanel_addr;
 
 /* Scan the system ROM for the signature "FJKEYINF" */
@@ -1159,6 +1180,7 @@ static void i801_probe_optional_slaves(struct i801_priv 
*priv)
 #else
 static void __init input_apanel_init(void) {}
 static void i801_probe_optional_slaves(struct i801_priv *priv) {}
+static void blacklist_features(struct i801_priv *priv) {}
 #endif /* CONFIG_X86 && CONFIG_DMI */
 
 #if IS_ENABLED(CONFIG_I2C_MUX_GPIO) && defined CONFIG_DMI
@@ -1562,6 +1584,7 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
   i801_feature_names[i]);
}
priv->features &= ~disable_features;
+   blacklist_features(priv);
 
err = pcim_enable_device(dev);
if (err) {
-- 
2.14.3

[PATCH v3] i2c: i801: blacklist Host Notify on HP EliteBook G3 850

[Jason Andryuk]

The HP EliteBook G3 850 has a weird bug where a subsequent cold boot
hangs while plugged in if Linux enables the Host Notify features of
i2c-i801.  The cold boot hang depends on how the system boots.  It does
not hang on UEFI Grub text boot or legacy Grub text boot.  But it does
hang on legacy Grub graphical boot and Intel Boot Agent PXE text boot.
Booting unplugged is not affected.

Disabling the Host Notify feature with disable_feature=0x20 works around
the bug, so automatically do so based on DMI information.

More information can be found here:
https://www.spinics.net/lists/linux-i2c/msg33938.html

Signed-off-by: Jason Andryuk 
Reviewed-by: Jean Delvare 
Cc: sta...@vger.kernel.org
---
v3: Switch to DMI_EXACT_MATCH and add empty element to array

 drivers/i2c/busses/i2c-i801.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 692b34125866..11149ddae745 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1042,6 +1042,27 @@ static const struct pci_device_id i801_ids[] = {
 MODULE_DEVICE_TABLE(pci, i801_ids);
 
 #if defined CONFIG_X86 && defined CONFIG_DMI
+static const struct dmi_system_id host_notify_dmi_blacklist[] = {
+   {
+   .ident = "HP EliteBook G3 850",
+   .matches = {
+   DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "HP"),
+   DMI_EXACT_MATCH(DMI_PRODUCT_NAME,
+   "HP EliteBook 850 G3"),
+   },
+   },
+   {}
+};
+
+static void blacklist_features(struct i801_priv *priv)
+{
+   if (dmi_check_system(host_notify_dmi_blacklist)) {
+   dev_warn(>pci_dev->dev,
+"SMBus Host Notify disabled on this system");
+   priv->features &= ~FEATURE_HOST_NOTIFY;
+   }
+}
+
 static unsigned char apanel_addr;
 
 /* Scan the system ROM for the signature "FJKEYINF" */
@@ -1159,6 +1180,7 @@ static void i801_probe_optional_slaves(struct i801_priv 
*priv)
 #else
 static void __init input_apanel_init(void) {}
 static void i801_probe_optional_slaves(struct i801_priv *priv) {}
+static void blacklist_features(struct i801_priv *priv) {}
 #endif /* CONFIG_X86 && CONFIG_DMI */
 
 #if IS_ENABLED(CONFIG_I2C_MUX_GPIO) && defined CONFIG_DMI
@@ -1562,6 +1584,7 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
   i801_feature_names[i]);
}
priv->features &= ~disable_features;
+   blacklist_features(priv);
 
err = pcim_enable_device(dev);
if (err) {
-- 
2.14.3

Re: [PATCH v2] i2c: i801: blacklist Host Notify on HP EliteBook G3 850

[Jason Andryuk]

On Tue, Apr 3, 2018 at 3:13 PM, Andreas Radke <andy...@archlinux.org> wrote:
> Am Mon,  2 Apr 2018 08:34:35 -0400
> schrieb Jason Andryuk <jandr...@gmail.com>:
>
>> The HP EliteBook G3 850 has a weird bug where a subsequent cold boot
>> hangs while plugged in if Linux enables the Host Notify features of
>> i2c-i801.  The cold boot hang depends on how the system boots.  It
>> does not hang on UEFI Grub text boot or legacy Grub text boot.  But
>> it does hang on legacy Grub graphical boot and Intel Boot Agent PXE
>> text boot. Booting unplugged is not affected.
>>
>> Disabling the Host Notify feature with disable_feature=0x20 works
>> around the bug, so automatically do so based on DMI information.
>>
>> More information can be found here:
>> https://www.spinics.net/lists/linux-i2c/msg33938.html
>
> I'm faced with similar cold boot issues on my HP ProBook 430 G4 model.
> Trying your hint with adding
> "options i2c_i801 disable_features=0x20"
> to /etc/modprobe.d/i2c_i801.conf doesn't seem to solve it for me.

Maybe verify the option is set by checking
/sys/module/i2c_i801/parameters/disable_features ?  Looks like dmesg
should also report "SMBus Host Notify disabled by user" when disabled.
I run with i801 built-in, so I was specifying the option on the kernel
command line.

> My system boots legacy grub in text mode (KMS) and seems to keep hanging
> even with that option set.

Your cold boot hang is in legacy grub itself?  Legacy grub text mode
actually doesn't hang my G3 850 - it was switching to the graphical
splash screen that seemed to do it.  PXE booting via F12 was another
consistent way to hang.  It was very early in the PXE boot code -
before it checked the link status.  So you could try that with it just
plugged into a switch even if you don't have a PXE boot setup.

>From my previous post:
"""
The Boot Agent hang prints "Client Mac Addr 00-11-22-33-44-55"
and hangs.  A normal boot prints a GUID on the same line and
continues.
"""

Otherwise, unfortunately, it might just be a different issue.

> Do you have any further idea?

For the G3 850, I bisected to the commit introducing i801 Host Notify
support.  It's time consuming, but you could do the same.  I first
noticed the issue when I moved from a working stable 4.4 to stable
4.14.  I tried some fedora kernels in between which narrowed down the
bisect range.  You could similarly grab and test pre-compiled kernels
to more quickly hone in on the regression range.

Regards,
Jason

Re: [PATCH v2] i2c: i801: blacklist Host Notify on HP EliteBook G3 850

[Jason Andryuk]

On Tue, Apr 3, 2018 at 3:13 PM, Andreas Radke  wrote:
> Am Mon,  2 Apr 2018 08:34:35 -0400
> schrieb Jason Andryuk :
>
>> The HP EliteBook G3 850 has a weird bug where a subsequent cold boot
>> hangs while plugged in if Linux enables the Host Notify features of
>> i2c-i801.  The cold boot hang depends on how the system boots.  It
>> does not hang on UEFI Grub text boot or legacy Grub text boot.  But
>> it does hang on legacy Grub graphical boot and Intel Boot Agent PXE
>> text boot. Booting unplugged is not affected.
>>
>> Disabling the Host Notify feature with disable_feature=0x20 works
>> around the bug, so automatically do so based on DMI information.
>>
>> More information can be found here:
>> https://www.spinics.net/lists/linux-i2c/msg33938.html
>
> I'm faced with similar cold boot issues on my HP ProBook 430 G4 model.
> Trying your hint with adding
> "options i2c_i801 disable_features=0x20"
> to /etc/modprobe.d/i2c_i801.conf doesn't seem to solve it for me.

Maybe verify the option is set by checking
/sys/module/i2c_i801/parameters/disable_features ?  Looks like dmesg
should also report "SMBus Host Notify disabled by user" when disabled.
I run with i801 built-in, so I was specifying the option on the kernel
command line.

> My system boots legacy grub in text mode (KMS) and seems to keep hanging
> even with that option set.

Your cold boot hang is in legacy grub itself?  Legacy grub text mode
actually doesn't hang my G3 850 - it was switching to the graphical
splash screen that seemed to do it.  PXE booting via F12 was another
consistent way to hang.  It was very early in the PXE boot code -
before it checked the link status.  So you could try that with it just
plugged into a switch even if you don't have a PXE boot setup.

>From my previous post:
"""
The Boot Agent hang prints "Client Mac Addr 00-11-22-33-44-55"
and hangs.  A normal boot prints a GUID on the same line and
continues.
"""

Otherwise, unfortunately, it might just be a different issue.

> Do you have any further idea?

For the G3 850, I bisected to the commit introducing i801 Host Notify
support.  It's time consuming, but you could do the same.  I first
noticed the issue when I moved from a working stable 4.4 to stable
4.14.  I tried some fedora kernels in between which narrowed down the
bisect range.  You could similarly grab and test pre-compiled kernels
to more quickly hone in on the regression range.

Regards,
Jason

[PATCH v2] i2c: i801: blacklist Host Notify on HP EliteBook G3 850

[Jason Andryuk]

The HP EliteBook G3 850 has a weird bug where a subsequent cold boot
hangs while plugged in if Linux enables the Host Notify features of
i2c-i801.  The cold boot hang depends on how the system boots.  It does
not hang on UEFI Grub text boot or legacy Grub text boot.  But it does
hang on legacy Grub graphical boot and Intel Boot Agent PXE text boot.
Booting unplugged is not affected.

Disabling the Host Notify feature with disable_feature=0x20 works around
the bug, so automatically do so based on DMI information.

More information can be found here:
https://www.spinics.net/lists/linux-i2c/msg33938.html

Signed-off-by: Jason Andryuk <jandr...@gmail.com>
---
I only added my machine to the blacklist, since it's the only one I've
tested.  More systems can be added when identified and tested.

v2: Fix open brace on function definition

 drivers/i2c/busses/i2c-i801.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 692b34125866..3e8e613c0801 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1042,6 +1042,25 @@ static const struct pci_device_id i801_ids[] = {
 MODULE_DEVICE_TABLE(pci, i801_ids);
 
 #if defined CONFIG_X86 && defined CONFIG_DMI
+static const struct dmi_system_id host_notify_dmi_blacklist[] = {
+   {
+   .ident = "HP EliteBook G3 850",
+   .matches = {
+   DMI_MATCH(DMI_BOARD_VENDOR, "HP"),
+   DMI_MATCH(DMI_PRODUCT_NAME, "HP EliteBook 850 G3"),
+   },
+   },
+};
+
+static void blacklist_features(struct i801_priv *priv)
+{
+   if (dmi_check_system(host_notify_dmi_blacklist)) {
+   dev_warn(>pci_dev->dev,
+"SMBus Host Notify disabled on this system");
+   priv->features &= ~FEATURE_HOST_NOTIFY;
+   }
+}
+
 static unsigned char apanel_addr;
 
 /* Scan the system ROM for the signature "FJKEYINF" */
@@ -1159,6 +1178,7 @@ static void i801_probe_optional_slaves(struct i801_priv 
*priv)
 #else
 static void __init input_apanel_init(void) {}
 static void i801_probe_optional_slaves(struct i801_priv *priv) {}
+static void blacklist_features(struct i801_priv *priv) {}
 #endif /* CONFIG_X86 && CONFIG_DMI */
 
 #if IS_ENABLED(CONFIG_I2C_MUX_GPIO) && defined CONFIG_DMI
@@ -1562,6 +1582,7 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
   i801_feature_names[i]);
}
priv->features &= ~disable_features;
+   blacklist_features(priv);
 
err = pcim_enable_device(dev);
if (err) {
-- 
2.14.3

[PATCH v2] i2c: i801: blacklist Host Notify on HP EliteBook G3 850

[Jason Andryuk]

The HP EliteBook G3 850 has a weird bug where a subsequent cold boot
hangs while plugged in if Linux enables the Host Notify features of
i2c-i801.  The cold boot hang depends on how the system boots.  It does
not hang on UEFI Grub text boot or legacy Grub text boot.  But it does
hang on legacy Grub graphical boot and Intel Boot Agent PXE text boot.
Booting unplugged is not affected.

Disabling the Host Notify feature with disable_feature=0x20 works around
the bug, so automatically do so based on DMI information.

More information can be found here:
https://www.spinics.net/lists/linux-i2c/msg33938.html

Signed-off-by: Jason Andryuk 
---
I only added my machine to the blacklist, since it's the only one I've
tested.  More systems can be added when identified and tested.

v2: Fix open brace on function definition

 drivers/i2c/busses/i2c-i801.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 692b34125866..3e8e613c0801 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1042,6 +1042,25 @@ static const struct pci_device_id i801_ids[] = {
 MODULE_DEVICE_TABLE(pci, i801_ids);
 
 #if defined CONFIG_X86 && defined CONFIG_DMI
+static const struct dmi_system_id host_notify_dmi_blacklist[] = {
+   {
+   .ident = "HP EliteBook G3 850",
+   .matches = {
+   DMI_MATCH(DMI_BOARD_VENDOR, "HP"),
+   DMI_MATCH(DMI_PRODUCT_NAME, "HP EliteBook 850 G3"),
+   },
+   },
+};
+
+static void blacklist_features(struct i801_priv *priv)
+{
+   if (dmi_check_system(host_notify_dmi_blacklist)) {
+   dev_warn(>pci_dev->dev,
+"SMBus Host Notify disabled on this system");
+   priv->features &= ~FEATURE_HOST_NOTIFY;
+   }
+}
+
 static unsigned char apanel_addr;
 
 /* Scan the system ROM for the signature "FJKEYINF" */
@@ -1159,6 +1178,7 @@ static void i801_probe_optional_slaves(struct i801_priv 
*priv)
 #else
 static void __init input_apanel_init(void) {}
 static void i801_probe_optional_slaves(struct i801_priv *priv) {}
+static void blacklist_features(struct i801_priv *priv) {}
 #endif /* CONFIG_X86 && CONFIG_DMI */
 
 #if IS_ENABLED(CONFIG_I2C_MUX_GPIO) && defined CONFIG_DMI
@@ -1562,6 +1582,7 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
   i801_feature_names[i]);
}
priv->features &= ~disable_features;
+   blacklist_features(priv);
 
err = pcim_enable_device(dev);
if (err) {
-- 
2.14.3

[PATCH] i2c: i801: blacklist Host Notify on HP EliteBook G3 850

[Jason Andryuk]

The HP EliteBook G3 850 has a weird bug where a subsequent cold boot
hangs while plugged in if Linux enables the Host Notify features of
i2c-i801.  The cold boot hang depends on how the system boots.  It does
not hang on UEFI Grub text boot or legacy Grub text boot.  But it does
hang on legacy Grub graphical boot and Intel Boot Agent PXE text boot.
Booting unplugged is not affected.

Disabling the Host Notify feature with disable_feature=0x20 works around
the bug, so automatically do so based on DMI information.

More information can be found here:
https://www.spinics.net/lists/linux-i2c/msg33938.html

Signed-off-by: Jason Andryuk <jandr...@gmail.com>
---
I only added my machine to the blacklist, since it's the only one I've
tested.  More systems can be added when identified and tested.

 drivers/i2c/busses/i2c-i801.c | 20 
 1 file changed, 20 insertions(+)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 692b34125866..2ff10c41ccad 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1042,6 +1042,24 @@ static const struct pci_device_id i801_ids[] = {
 MODULE_DEVICE_TABLE(pci, i801_ids);
 
 #if defined CONFIG_X86 && defined CONFIG_DMI
+static const struct dmi_system_id host_notify_dmi_blacklist[] = {
+   {
+   .ident = "HP EliteBook G3 850",
+   .matches = {
+   DMI_MATCH(DMI_BOARD_VENDOR, "HP"),
+   DMI_MATCH(DMI_PRODUCT_NAME, "HP EliteBook 850 G3"),
+   },
+   },
+};
+
+static void blacklist_features(struct i801_priv *priv) {
+   if (dmi_check_system(host_notify_dmi_blacklist)) {
+   dev_warn(>pci_dev->dev,
+"SMBus Host Notify disabled on this system");
+   priv->features &= ~FEATURE_HOST_NOTIFY;
+   }
+}
+
 static unsigned char apanel_addr;
 
 /* Scan the system ROM for the signature "FJKEYINF" */
@@ -1159,6 +1177,7 @@ static void i801_probe_optional_slaves(struct i801_priv 
*priv)
 #else
 static void __init input_apanel_init(void) {}
 static void i801_probe_optional_slaves(struct i801_priv *priv) {}
+static void blacklist_features(struct i801_priv *priv) {}
 #endif /* CONFIG_X86 && CONFIG_DMI */
 
 #if IS_ENABLED(CONFIG_I2C_MUX_GPIO) && defined CONFIG_DMI
@@ -1562,6 +1581,7 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
   i801_feature_names[i]);
}
priv->features &= ~disable_features;
+   blacklist_features(priv);
 
err = pcim_enable_device(dev);
if (err) {
-- 
2.14.3

[PATCH] i2c: i801: blacklist Host Notify on HP EliteBook G3 850

[Jason Andryuk]

The HP EliteBook G3 850 has a weird bug where a subsequent cold boot
hangs while plugged in if Linux enables the Host Notify features of
i2c-i801.  The cold boot hang depends on how the system boots.  It does
not hang on UEFI Grub text boot or legacy Grub text boot.  But it does
hang on legacy Grub graphical boot and Intel Boot Agent PXE text boot.
Booting unplugged is not affected.

Disabling the Host Notify feature with disable_feature=0x20 works around
the bug, so automatically do so based on DMI information.

More information can be found here:
https://www.spinics.net/lists/linux-i2c/msg33938.html

Signed-off-by: Jason Andryuk 
---
I only added my machine to the blacklist, since it's the only one I've
tested.  More systems can be added when identified and tested.

 drivers/i2c/busses/i2c-i801.c | 20 
 1 file changed, 20 insertions(+)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 692b34125866..2ff10c41ccad 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1042,6 +1042,24 @@ static const struct pci_device_id i801_ids[] = {
 MODULE_DEVICE_TABLE(pci, i801_ids);
 
 #if defined CONFIG_X86 && defined CONFIG_DMI
+static const struct dmi_system_id host_notify_dmi_blacklist[] = {
+   {
+   .ident = "HP EliteBook G3 850",
+   .matches = {
+   DMI_MATCH(DMI_BOARD_VENDOR, "HP"),
+   DMI_MATCH(DMI_PRODUCT_NAME, "HP EliteBook 850 G3"),
+   },
+   },
+};
+
+static void blacklist_features(struct i801_priv *priv) {
+   if (dmi_check_system(host_notify_dmi_blacklist)) {
+   dev_warn(>pci_dev->dev,
+"SMBus Host Notify disabled on this system");
+   priv->features &= ~FEATURE_HOST_NOTIFY;
+   }
+}
+
 static unsigned char apanel_addr;
 
 /* Scan the system ROM for the signature "FJKEYINF" */
@@ -1159,6 +1177,7 @@ static void i801_probe_optional_slaves(struct i801_priv 
*priv)
 #else
 static void __init input_apanel_init(void) {}
 static void i801_probe_optional_slaves(struct i801_priv *priv) {}
+static void blacklist_features(struct i801_priv *priv) {}
 #endif /* CONFIG_X86 && CONFIG_DMI */
 
 #if IS_ENABLED(CONFIG_I2C_MUX_GPIO) && defined CONFIG_DMI
@@ -1562,6 +1581,7 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
   i801_feature_names[i]);
}
priv->features &= ~disable_features;
+   blacklist_features(priv);
 
err = pcim_enable_device(dev);
if (err) {
-- 
2.14.3

Re: [PATCH] x86/xen: Delay get_cpu_cap until stack canary is established

[Jason Andryuk]

On Sat, Mar 31, 2018 at 2:10 PM, Boris Ostrovsky
<boris.ostrov...@oracle.com> wrote:
> On 03/31/2018 01:38 PM, Jason Andryuk wrote:
>> On Wed, Mar 21, 2018, 5:12 PM Boris Ostrovsky <boris.ostrov...@oracle.com
>> <mailto:boris.ostrov...@oracle.com>> wrote:
>>
>> On 03/19/2018 12:58 PM, Jason Andryuk wrote:
>>  > Commit 2cc42bac1c79 ("x86-64/Xen: eliminate W+X mappings")
>> introduced a
>>  > call to get_cpu_cap, which is fstack-protected.  This is works on
>> x86-64
>>  > as commit 4f277295e54c ("x86/xen: init %gs very early to avoid page
>>  > faults with stack protector") ensures the stack protector is
>> configured,
>>  > but it it did not cover x86-32.
>>  >
>>  > Delay calling get_cpu_cap until after xen_setup_gdt has
>> initialized the
>>  > stack canary.  Without this, a 32bit PV machine crashes early
>>  > in boot.
>>  > (XEN) Domain 0 (vcpu#0) crashed on cpu#0:
>>  > (XEN) [ Xen-4.6.6-xc  x86_64  debug=n  Tainted:C ]
>>  > (XEN) CPU:0
>>  > (XEN) RIP:e019:[<c10362f8>]
>>  >
>>  > And the PV kernel IP corresponds to init_scattered_cpuid_features
>>  >0xc10362f8 <+24>:mov%gs:0x14,%eax
>>  >
>>  > Fixes 2cc42bac1c79 ("x86-64/Xen: eliminate W+X mappings")
>>  >
>>  > Signed-off-by: Jason Andryuk <jandr...@gmail.com
>> <mailto:jandr...@gmail.com>>
>>  >
>>
>>
>> Applied to for-linus-4.17
>>
>>
>> Thanks. If it's not too late, can this be cc: stable?
>
> We can always try ;-)
>
> This is 4.15 and 4.16 only, I believe.

I'm using this patch on 4.14, so there as well.

-Jason

Re: [PATCH] x86/xen: Delay get_cpu_cap until stack canary is established

[Jason Andryuk]

On Sat, Mar 31, 2018 at 2:10 PM, Boris Ostrovsky
 wrote:
> On 03/31/2018 01:38 PM, Jason Andryuk wrote:
>> On Wed, Mar 21, 2018, 5:12 PM Boris Ostrovsky > <mailto:boris.ostrov...@oracle.com>> wrote:
>>
>>     On 03/19/2018 12:58 PM, Jason Andryuk wrote:
>>  > Commit 2cc42bac1c79 ("x86-64/Xen: eliminate W+X mappings")
>> introduced a
>>  > call to get_cpu_cap, which is fstack-protected.  This is works on
>> x86-64
>>  > as commit 4f277295e54c ("x86/xen: init %gs very early to avoid page
>>  > faults with stack protector") ensures the stack protector is
>> configured,
>>  > but it it did not cover x86-32.
>>  >
>>  > Delay calling get_cpu_cap until after xen_setup_gdt has
>> initialized the
>>  > stack canary.  Without this, a 32bit PV machine crashes early
>>  > in boot.
>>  > (XEN) Domain 0 (vcpu#0) crashed on cpu#0:
>>  > (XEN) [ Xen-4.6.6-xc  x86_64  debug=n  Tainted:C ]
>>  > (XEN) CPU:0
>>  > (XEN) RIP:e019:[<c10362f8>]
>>  >
>>  > And the PV kernel IP corresponds to init_scattered_cpuid_features
>>  >0xc10362f8 <+24>:mov%gs:0x14,%eax
>>  >
>>  > Fixes 2cc42bac1c79 ("x86-64/Xen: eliminate W+X mappings")
>>  >
>>  > Signed-off-by: Jason Andryuk > <mailto:jandr...@gmail.com>>
>>  >
>>
>>
>> Applied to for-linus-4.17
>>
>>
>> Thanks. If it's not too late, can this be cc: stable?
>
> We can always try ;-)
>
> This is 4.15 and 4.16 only, I believe.

I'm using this patch on 4.14, so there as well.

-Jason

[PATCH] x86/xen: Delay get_cpu_cap until stack canary is established

[Jason Andryuk]

Commit 2cc42bac1c79 ("x86-64/Xen: eliminate W+X mappings") introduced a
call to get_cpu_cap, which is fstack-protected.  This is works on x86-64
as commit 4f277295e54c ("x86/xen: init %gs very early to avoid page
faults with stack protector") ensures the stack protector is configured,
but it it did not cover x86-32.

Delay calling get_cpu_cap until after xen_setup_gdt has initialized the
stack canary.  Without this, a 32bit PV machine crashes early
in boot.
(XEN) Domain 0 (vcpu#0) crashed on cpu#0:
(XEN) [ Xen-4.6.6-xc  x86_64  debug=n  Tainted:C ]
(XEN) CPU:0
(XEN) RIP:e019:[<c10362f8>]

And the PV kernel IP corresponds to init_scattered_cpuid_features
   0xc10362f8 <+24>:mov%gs:0x14,%eax

Fixes 2cc42bac1c79 ("x86-64/Xen: eliminate W+X mappings")

Signed-off-by: Jason Andryuk <jandr...@gmail.com>
---
 arch/x86/xen/enlighten_pv.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index 3c2c2530737e..c36d23aa6c35 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -1259,10 +1259,6 @@ asmlinkage __visible void __init xen_start_kernel(void)
 */
__userpte_alloc_gfp &= ~__GFP_HIGHMEM;
 
-   /* Work out if we support NX */
-   get_cpu_cap(_cpu_data);
-   x86_configure_nx();
-
/* Get mfn list */
xen_build_dynamic_phys_to_machine();
 
@@ -1272,6 +1268,10 @@ asmlinkage __visible void __init xen_start_kernel(void)
 */
xen_setup_gdt(0);
 
+   /* Work out if we support NX */
+   get_cpu_cap(_cpu_data);
+   x86_configure_nx();
+
xen_init_irq_ops();
 
/* Let's presume PV guests always boot on vCPU with id 0. */
-- 
2.14.3

[PATCH] x86/xen: Delay get_cpu_cap until stack canary is established

[Jason Andryuk]

Commit 2cc42bac1c79 ("x86-64/Xen: eliminate W+X mappings") introduced a
call to get_cpu_cap, which is fstack-protected.  This is works on x86-64
as commit 4f277295e54c ("x86/xen: init %gs very early to avoid page
faults with stack protector") ensures the stack protector is configured,
but it it did not cover x86-32.

Delay calling get_cpu_cap until after xen_setup_gdt has initialized the
stack canary.  Without this, a 32bit PV machine crashes early
in boot.
(XEN) Domain 0 (vcpu#0) crashed on cpu#0:
(XEN) [ Xen-4.6.6-xc  x86_64  debug=n  Tainted:C ]
(XEN) CPU:0
(XEN) RIP:e019:[<c10362f8>]

And the PV kernel IP corresponds to init_scattered_cpuid_features
   0xc10362f8 <+24>:mov%gs:0x14,%eax

Fixes 2cc42bac1c79 ("x86-64/Xen: eliminate W+X mappings")

Signed-off-by: Jason Andryuk 
---
 arch/x86/xen/enlighten_pv.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index 3c2c2530737e..c36d23aa6c35 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -1259,10 +1259,6 @@ asmlinkage __visible void __init xen_start_kernel(void)
 */
__userpte_alloc_gfp &= ~__GFP_HIGHMEM;
 
-   /* Work out if we support NX */
-   get_cpu_cap(_cpu_data);
-   x86_configure_nx();
-
/* Get mfn list */
xen_build_dynamic_phys_to_machine();
 
@@ -1272,6 +1268,10 @@ asmlinkage __visible void __init xen_start_kernel(void)
 */
xen_setup_gdt(0);
 
+   /* Work out if we support NX */
+   get_cpu_cap(_cpu_data);
+   x86_configure_nx();
+
xen_init_irq_ops();
 
/* Let's presume PV guests always boot on vCPU with id 0. */
-- 
2.14.3

[PATCH] xen-netfront: Fix hang on device removal

[Jason Andryuk]

A toolstack may delete the vif frontend and backend xenstore entries
while xen-netfront is in the removal code path.  In that case, the
checks for xenbus_read_driver_state would return XenbusStateUnknown, and
xennet_remove would hang indefinitely.  This hang prevents system
shutdown.

xennet_remove must be able to handle XenbusStateUnknown, and
netback_changed must also wake up the wake_queue for that state as well.

Fixes: 5b5971df3bc2 ("xen-netfront: remove warning when unloading module")

Signed-off-by: Jason Andryuk <jandr...@gmail.com>
Cc: Eduardo Otubo <ot...@redhat.com>
---
 drivers/net/xen-netfront.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 8328d395e332..3127bc8633ca 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -2005,7 +2005,10 @@ static void netback_changed(struct xenbus_device *dev,
case XenbusStateInitialised:
case XenbusStateReconfiguring:
case XenbusStateReconfigured:
+   break;
+
case XenbusStateUnknown:
+   wake_up_all(_unload_q);
break;
 
case XenbusStateInitWait:
@@ -2136,7 +2139,9 @@ static int xennet_remove(struct xenbus_device *dev)
xenbus_switch_state(dev, XenbusStateClosing);
wait_event(module_unload_q,
   xenbus_read_driver_state(dev->otherend) ==
-  XenbusStateClosing);
+  XenbusStateClosing ||
+  xenbus_read_driver_state(dev->otherend) ==
+  XenbusStateUnknown);
 
xenbus_switch_state(dev, XenbusStateClosed);
wait_event(module_unload_q,
-- 
2.14.3

[PATCH] xen-netfront: Fix hang on device removal

[Jason Andryuk]

A toolstack may delete the vif frontend and backend xenstore entries
while xen-netfront is in the removal code path.  In that case, the
checks for xenbus_read_driver_state would return XenbusStateUnknown, and
xennet_remove would hang indefinitely.  This hang prevents system
shutdown.

xennet_remove must be able to handle XenbusStateUnknown, and
netback_changed must also wake up the wake_queue for that state as well.

Fixes: 5b5971df3bc2 ("xen-netfront: remove warning when unloading module")

Signed-off-by: Jason Andryuk 
Cc: Eduardo Otubo 
---
 drivers/net/xen-netfront.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 8328d395e332..3127bc8633ca 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -2005,7 +2005,10 @@ static void netback_changed(struct xenbus_device *dev,
case XenbusStateInitialised:
case XenbusStateReconfiguring:
case XenbusStateReconfigured:
+   break;
+
case XenbusStateUnknown:
+   wake_up_all(_unload_q);
break;
 
case XenbusStateInitWait:
@@ -2136,7 +2139,9 @@ static int xennet_remove(struct xenbus_device *dev)
xenbus_switch_state(dev, XenbusStateClosing);
wait_event(module_unload_q,
   xenbus_read_driver_state(dev->otherend) ==
-  XenbusStateClosing);
+  XenbusStateClosing ||
+  xenbus_read_driver_state(dev->otherend) ==
+  XenbusStateUnknown);
 
xenbus_switch_state(dev, XenbusStateClosed);
wait_event(module_unload_q,
-- 
2.14.3

[PATCH] lib/ucs2_string: Correct ucs2 -> utf8 conversion

[Jason Andryuk]

The comparisons should be >= since 0x800 and 0x80 require an additional bit
to store.

For the 3 byte case, the existing shift would drop off 2 more bits than
intended.

For the 2 byte case, there should be 5 bits bits in byte 1, and 6 bits in
byte 2.

Signed-off-by: Jason Andryuk 
---

Tested in user space, but not in the kernel.  Conversions now match
python's unicode conversions.

 lib/ucs2_string.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/ucs2_string.c b/lib/ucs2_string.c
index 17dd74e..f0b323a 100644
--- a/lib/ucs2_string.c
+++ b/lib/ucs2_string.c
@@ -59,9 +59,9 @@ ucs2_utf8size(const ucs2_char_t *src)
for (i = 0; i < ucs2_strlen(src); i++) {
u16 c = src[i];
 
-   if (c > 0x800)
+   if (c >= 0x800)
j += 3;
-   else if (c > 0x80)
+   else if (c >= 0x80)
j += 2;
else
j += 1;
@@ -88,19 +88,19 @@ ucs2_as_utf8(u8 *dest, const ucs2_char_t *src, unsigned 
long maxlength)
for (i = 0; maxlength && i < limit; i++) {
u16 c = src[i];
 
-   if (c > 0x800) {
+   if (c >= 0x800) {
if (maxlength < 3)
break;
maxlength -= 3;
dest[j++] = 0xe0 | (c & 0xf000) >> 12;
-   dest[j++] = 0x80 | (c & 0x0fc0) >> 8;
+   dest[j++] = 0x80 | (c & 0x0fc0) >> 6;
dest[j++] = 0x80 | (c & 0x003f);
-   } else if (c > 0x80) {
+   } else if (c >= 0x80) {
if (maxlength < 2)
break;
maxlength -= 2;
-   dest[j++] = 0xc0 | (c & 0xfe0) >> 5;
-   dest[j++] = 0x80 | (c & 0x01f);
+   dest[j++] = 0xc0 | (c & 0x7c0) >> 6;
+   dest[j++] = 0x80 | (c & 0x03f);
} else {
maxlength -= 1;
dest[j++] = c & 0x7f;
-- 
2.4.3

[PATCH] lib/ucs2_string: Correct ucs2 -> utf8 conversion

[Jason Andryuk]

The comparisons should be >= since 0x800 and 0x80 require an additional bit
to store.

For the 3 byte case, the existing shift would drop off 2 more bits than
intended.

For the 2 byte case, there should be 5 bits bits in byte 1, and 6 bits in
byte 2.

Signed-off-by: Jason Andryuk <jandr...@gmail.com>
---

Tested in user space, but not in the kernel.  Conversions now match
python's unicode conversions.

 lib/ucs2_string.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/ucs2_string.c b/lib/ucs2_string.c
index 17dd74e..f0b323a 100644
--- a/lib/ucs2_string.c
+++ b/lib/ucs2_string.c
@@ -59,9 +59,9 @@ ucs2_utf8size(const ucs2_char_t *src)
for (i = 0; i < ucs2_strlen(src); i++) {
u16 c = src[i];
 
-   if (c > 0x800)
+   if (c >= 0x800)
j += 3;
-   else if (c > 0x80)
+   else if (c >= 0x80)
j += 2;
else
j += 1;
@@ -88,19 +88,19 @@ ucs2_as_utf8(u8 *dest, const ucs2_char_t *src, unsigned 
long maxlength)
for (i = 0; maxlength && i < limit; i++) {
u16 c = src[i];
 
-   if (c > 0x800) {
+   if (c >= 0x800) {
if (maxlength < 3)
break;
maxlength -= 3;
dest[j++] = 0xe0 | (c & 0xf000) >> 12;
-   dest[j++] = 0x80 | (c & 0x0fc0) >> 8;
+   dest[j++] = 0x80 | (c & 0x0fc0) >> 6;
dest[j++] = 0x80 | (c & 0x003f);
-   } else if (c > 0x80) {
+   } else if (c >= 0x80) {
if (maxlength < 2)
break;
maxlength -= 2;
-   dest[j++] = 0xc0 | (c & 0xfe0) >> 5;
-   dest[j++] = 0x80 | (c & 0x01f);
+   dest[j++] = 0xc0 | (c & 0x7c0) >> 6;
+   dest[j++] = 0x80 | (c & 0x03f);
} else {
maxlength -= 1;
dest[j++] = c & 0x7f;
-- 
2.4.3