2020-09-10 21:07:46

by Boqun Feng

Subject: [PATCH v3 08/11] Input: hyperv-keyboard: Make ringbuffer at least take two pages

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 <[email protected]>
---
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))

#define XTKBD_EMUL0 0xe0
#define XTKBD_EMUL1 0xe1
--
2.28.0

2020-09-12 19:40:35

by Michael Kelley (LINUX)

Subject: RE: [PATCH v3 08/11] Input: hyperv-keyboard: Make ringbuffer at least take two pages

From: Boqun Feng <[email protected]> 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 <[email protected]>
> ---
> drivers/input/serio/hyperv-keyboard.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>

Reviewed-by: Michael Kelley <[email protected]>

2020-09-13 17:01:41

by Dmitry Torokhov

Subject: Re: [PATCH v3 08/11] Input: hyperv-keyboard: Make ringbuffer at least take two pages

On Sat, Sep 12, 2020 at 07:37:23PM +0000, Michael Kelley wrote:
> From: Boqun Feng <[email protected]> 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 <[email protected]>
> > ---
> > drivers/input/serio/hyperv-keyboard.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
>
> Reviewed-by: Michael Kelley <[email protected]>

Acked-by: Dmitry Torokhov <[email protected]>

Please feel free to merge with the rest of the patches through whatever
tree they will go in.

Thanks.

--
Dmitry

2020-09-14 08:47:18

by Boqun Feng

Subject: Re: [PATCH v3 08/11] Input: hyperv-keyboard: Make ringbuffer at least take two pages

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 <[email protected]>
> ---
> 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.

Thoughts?

Regards,
Boqun

> #define XTKBD_EMUL0 0xe0
> #define XTKBD_EMUL1 0xe1
> --
> 2.28.0
>

2020-09-14 09:32:36

by Wei Liu

Subject: Re: [PATCH v3 08/11] Input: hyperv-keyboard: Make ringbuffer at least take two pages

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 <[email protected]>
> > ---
> > 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?

Wei.

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

2020-09-14 10:24:08

by Boqun Feng

Subject: Re: [PATCH v3 08/11] Input: hyperv-keyboard: Make ringbuffer at least take two pages

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 <[email protected]>
> > > ---
> > > 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
> > >