mbox series

[v3,00/11] Hyper-V: Support PAGE_SIZE larger than 4K

Message ID 20200910143455.109293-1-boqun.feng@gmail.com
Headers show
Series Hyper-V: Support PAGE_SIZE larger than 4K | expand

Message

Boqun Feng Sept. 10, 2020, 2:34 p.m. UTC 
This patchset add the necessary changes to support guests whose page
size is larger than 4K. And the main architecture which we develop this
for is ARM64 (also it's the architecture that I use to test this
feature). Now as the patchset has been deeply reviewed, I change it from
"RFC" to "PATCH", and also add ARM64 people Cced for broader insights
(although the code is arch-independent).

Previous version:
v1: https://lore.kernel.org/lkml/20200721014135.84140-1-boqun.feng@gmail.com/
v2: https://lore.kernel.org/lkml/20200902030107.33380-1-boqun.feng@gmail.com

Changes since v2:

*	Use a simpler and straight-forwards method to set up the payload
	array for storvsc thanks to the inspiration from Michael Kelley.

*	Some typo fixes as per suggestion from Michael Kelley.

*	Fixes compiler warnings due to the different types of two
	operands for max().


Hyper-V always uses 4K as the page size and expects the same page size
when communicating with guests. That is, all the "pfn"s in the
hypervisor-guest communication protocol are the page numbers in the unit
of HV_HYP_PAGE_SIZE rather than PAGE_SIZE. To support guests with larger
page size, we need to convert between these two page sizes correctly in
the hypervisor-guest communication, which is basically what this
patchset does.

In this conversion, one challenge is how to handle the ringbuffer. A
ringbuffer has two parts: a header and a data part, both of which want
to be PAGE_SIZE aligned in the guest, because we use the "double
mapping" trick to map the data part twice in the guest virtual address
space for faster wrap-around and ease to process data in place. However,
the Hyper-V hypervisor always treats the ringbuffer headers as 4k pages.
To overcome this gap, we enlarge the hv_ring_buffer structure to be
always PAGE_SIZE aligned, and introduce the gpadl type concept to allow
vmbus_establish_gpadl() to handle ringbuffer cases specially. Note that
gpadl type is only meaningful to the guest, there is no such concept in
Hyper-V hypervisor.

This patchset consists of 11 patches:

Patch 1~4: Introduce the types of gpadl, so that we can handle
	   ringbuffer when PAGE_SIZE != HV_HYP_PAGE_SIZE, and also fix
	   a few places where we should use HV_HYP_PAGE_SIZE other than
	   PAGE_SIZE.

Patch 5~6: Add a few helper functions to help calculate the hvpfn (page
	   number in the unit of HV_HYP_PAGE_SIZE) and other related
	   data. So that we can use them in the code of drivers.

Patch 7~11: Use the helpers and change the driver code accordingly to
	    make net/input/util/storage driver work with PAGE_SIZE !=
	    HV_HYP_PAGE_SIZE

I've done some tests with PAGE_SIZE=64k and PAGE_SIZE=16k configurations
on ARM64 guests (with Michael's patchset[1] for ARM64 Hyper-V guest
support), everything worked fine ;-) (I could observe an error caused
by unaligned firmware data, but it's better to have it fixed in the
Hyper-V). I also have done a build and boot test on x86, everything
worked well.

Looking forwards to comments and suggestions!

Regards,
Boqun

[1]: https://lore.kernel.org/lkml/1598287583-71762-1-git-send-email-mikelley@microsoft.com/

Boqun Feng (11):
  Drivers: hv: vmbus: Always use HV_HYP_PAGE_SIZE for gpadl
  Drivers: hv: vmbus: Move __vmbus_open()
  Drivers: hv: vmbus: Introduce types of GPADL
  Drivers: hv: Use HV_HYP_PAGE in hv_synic_enable_regs()
  Drivers: hv: vmbus: Move virt_to_hvpfn() to hyperv header
  hv: hyperv.h: Introduce some hvpfn helper functions
  hv_netvsc: Use HV_HYP_PAGE_SIZE for Hyper-V communication
  Input: hyperv-keyboard: Make ringbuffer at least take two pages
  HID: hyperv: Make ringbuffer at least take two pages
  Driver: hv: util: Make ringbuffer at least take two pages
  scsi: storvsc: Support PAGE_SIZE larger than 4K

 drivers/hid/hid-hyperv.c              |   4 +-
 drivers/hv/channel.c                  | 461 ++++++++++++++++----------
 drivers/hv/hv.c                       |   4 +-
 drivers/hv/hv_util.c                  |  16 +-
 drivers/input/serio/hyperv-keyboard.c |   4 +-
 drivers/net/hyperv/netvsc.c           |   2 +-
 drivers/net/hyperv/netvsc_drv.c       |  46 +--
 drivers/net/hyperv/rndis_filter.c     |  13 +-
 drivers/scsi/storvsc_drv.c            |  54 ++-
 include/linux/hyperv.h                |  64 +++-
 10 files changed, 441 insertions(+), 227 deletions(-)

Comments

Michael Kelley Sept. 12, 2020, 7:24 p.m. UTC | #1 
From: Boqun Feng <boqun.feng@gmail.com> Sent: Thursday, September 10, 2020 7:35 AM
> 
> Pure function movement, no functional changes. The move is made, because
> in a later change, __vmbus_open() will rely on some static functions
> afterwards, so we separate the move and the modification of
> __vmbus_open() in two patches to make it easy to review.
> 
> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> Reviewed-by: Wei Liu <wei.liu@kernel.org>
> ---
>  drivers/hv/channel.c | 309 ++++++++++++++++++++++---------------------
>  1 file changed, 155 insertions(+), 154 deletions(-)
> 

Reviewed-by: Michael Kelley <mikelley@microsoft.com>
Michael Kelley Sept. 12, 2020, 7:30 p.m. UTC | #2 
From: Boqun Feng <boqun.feng@gmail.com> Sent: Thursday, September 10, 2020 7:35 AM
> 
> Both the base_*_gpa should use the guest page number in Hyper-V page, so
> use HV_HYP_PAGE instead of PAGE.
> 
> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> ---
>  drivers/hv/hv.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

Reviewed-by: Michael Kelley <mikelley@microsoft.com>
Michael Kelley Sept. 12, 2020, 7:37 p.m. UTC | #3 
From: Boqun Feng <boqun.feng@gmail.com> Sent: Thursday, September 10, 2020 7:35 AM

> 
> When PAGE_SIZE > HV_HYP_PAGE_SIZE, we need the ringbuffer size to be at
> least 2 * PAGE_SIZE: one page for the header and at least one page of
> the data part (because of the alignment requirement for double mapping).
> 
> So make sure the ringbuffer sizes to be at least 2 * PAGE_SIZE when
> using vmbus_open() to establish the vmbus connection.
> 
> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> ---
>  drivers/input/serio/hyperv-keyboard.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

Reviewed-by: Michael Kelley <mikelley@microsoft.com>
Boqun Feng Sept. 14, 2020, 10:22 a.m. UTC | #4 
On Mon, Sep 14, 2020 at 09:30:16AM +0000, Wei Liu wrote:
> On Mon, Sep 14, 2020 at 04:46:00PM +0800, Boqun Feng wrote:
> > On Thu, Sep 10, 2020 at 10:34:52PM +0800, Boqun Feng wrote:
> > > When PAGE_SIZE > HV_HYP_PAGE_SIZE, we need the ringbuffer size to be at
> > > least 2 * PAGE_SIZE: one page for the header and at least one page of
> > > the data part (because of the alignment requirement for double mapping).
> > > 
> > > So make sure the ringbuffer sizes to be at least 2 * PAGE_SIZE when
> > > using vmbus_open() to establish the vmbus connection.
> > > 
> > > Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> > > ---
> > >  drivers/input/serio/hyperv-keyboard.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/input/serio/hyperv-keyboard.c b/drivers/input/serio/hyperv-keyboard.c
> > > index df4e9f6f4529..6ebc61e2db3f 100644
> > > --- a/drivers/input/serio/hyperv-keyboard.c
> > > +++ b/drivers/input/serio/hyperv-keyboard.c
> > > @@ -75,8 +75,8 @@ struct synth_kbd_keystroke {
> > >  
> > >  #define HK_MAXIMUM_MESSAGE_SIZE 256
> > >  
> > > -#define KBD_VSC_SEND_RING_BUFFER_SIZE		(40 * 1024)
> > > -#define KBD_VSC_RECV_RING_BUFFER_SIZE		(40 * 1024)
> > > +#define KBD_VSC_SEND_RING_BUFFER_SIZE	max(40 * 1024, (int)(2 * PAGE_SIZE))
> > > +#define KBD_VSC_RECV_RING_BUFFER_SIZE	max(40 * 1024, (int)(2 * PAGE_SIZE))
> > >  
> > 
> > Hmm.. just realized there is a problem here, if PAGE_SIZE = 16k, then
> > 40 * 1024 > 2 * PAGE_SIZE, however in the ring buffer size should also
> > be page aligned, otherwise vmbus_open() will fail.
> > 
> > I plan to modify this as
> > 
> > in linux/hyperv.h:
> > 
> > #define VMBUS_RING_SIZE(payload_sz) PAGE_ALIGN(sizeof(struct hv_ring_buffer) + (playload_sz))
> > 
> > and here:
> > 
> > #define KBD_VSC_SEND_RING_BUFFER_SIZE VMBUS_RING_SIZE(36 * 1024)
> > #define KBD_VSC_RECV_RING_BUFFER_SIZE VMBUS_RING_SIZE(36 * 1024)
> > 
> > and the similar change for patch #9.
> 
> OOI why do you reduce the size by 4k here?
> 

To keep the total ring buffer size unchanged (still 40k) when
PAGE_SIZE=4k. Because in VMBUS_RING_SIZE() (which I plan to rename as
HV_RING_SIZE()), the hv_ring_buffer size is already added.

Regards,
Boqun

> Wei.
> 
> > 
> > Thoughts?
> > 
> > Regards,
> > Boqun
> > 
> > >  #define XTKBD_EMUL0     0xe0
> > >  #define XTKBD_EMUL1     0xe1
> > > -- 
> > > 2.28.0
> > >