mbox series

[v13,00/21] KVM: xen: update shared_info and vcpu_info handling

Message ID 20240215152916.1158-1-paul@xen.org
Headers
Series KVM: xen: update shared_info and vcpu_info handling |

Message

Paul Durrant Feb. 15, 2024, 3:28 p.m. UTC 
  From: Paul Durrant <pdurrant@amazon.com>

This series contains a new patch from Sean added since v12 [1]:

* KVM: s390: Refactor kvm_is_error_gpa() into kvm_is_gpa_in_memslot()

This frees up the function name kvm_is_error_gpa() such that it can then be
re-defined in:

* KVM: pfncache: allow a cache to be activated with a fixed (userspace) HVA

to be used for a simple GPA validation helper function. The patch also now
contains an either/or address check for GPA versus HVA in
__kvm_gpc_refresh().

In:

* KVM: pfncache: add a mark-dirty helper

The function name has been changed from kvm_gpc_mark_dirty() to
kvm_gpc_mark_dirty_in_slot().

In:

* KVM: x86/xen: allow shared_info to be mapped by fixed HVA

missing HVA validation checks have been added and the 'hva == 0' test
has been changed to '!hva'. The KVM_XEN_ATTR_TYPE_SHARED_INFO and
KVM_XEN_ATTR_TYPE_SHARED_INFO_HVA cases are still largely handled as one
though as separation leads to duplicate calls to
kvm_xen_shared_info_init() which looks messy.

Also, patches with a 'xen' prefix have now been modified to have a
'x86/xen' prefix and patches with a 'selftests / xen' prefix have been
modified to have simply a 'selftests' prefix.

[1] https://lore.kernel.org/kvm/20240115125707.1183-1-paul@xen.org/

David Woodhouse (1):
  KVM: pfncache: rework __kvm_gpc_refresh() to fix locking issues

Paul Durrant (19):
  KVM: pfncache: Add a map helper function
  KVM: pfncache: remove unnecessary exports
  KVM: x86/xen: mark guest pages dirty with the pfncache lock held
  KVM: pfncache: add a mark-dirty helper
  KVM: pfncache: remove KVM_GUEST_USES_PFN usage
  KVM: pfncache: stop open-coding offset_in_page()
  KVM: pfncache: include page offset in uhva and use it consistently
  KVM: pfncache: allow a cache to be activated with a fixed (userspace)
    HVA
  KVM: x86/xen: separate initialization of shared_info cache and content
  KVM: x86/xen: re-initialize shared_info if guest (32/64-bit) mode is
    set
  KVM: x86/xen: allow shared_info to be mapped by fixed HVA
  KVM: x86/xen: allow vcpu_info to be mapped by fixed HVA
  KVM: selftests: map Xen's shared_info page using HVA rather than GFN
  KVM: selftests: re-map Xen's vcpu_info using HVA rather than GPA
  KVM: x86/xen: advertize the KVM_XEN_HVM_CONFIG_SHARED_INFO_HVA
    capability
  KVM: x86/xen: split up kvm_xen_set_evtchn_fast()
  KVM: x86/xen: don't block on pfncache locks in
    kvm_xen_set_evtchn_fast()
  KVM: pfncache: check the need for invalidation under read lock first
  KVM: x86/xen: allow vcpu_info content to be 'safely' copied

Sean Christopherson (1):
  KVM: s390: Refactor kvm_is_error_gpa() into kvm_is_gpa_in_memslot()

 Documentation/virt/kvm/api.rst                |  53 ++-
 arch/s390/kvm/diag.c                          |   2 +-
 arch/s390/kvm/gaccess.c                       |  14 +-
 arch/s390/kvm/kvm-s390.c                      |   4 +-
 arch/s390/kvm/priv.c                          |   4 +-
 arch/s390/kvm/sigp.c                          |   2 +-
 arch/x86/kvm/x86.c                            |   7 +-
 arch/x86/kvm/xen.c                            | 361 +++++++++++------
 include/linux/kvm_host.h                      |  49 ++-
 include/linux/kvm_types.h                     |   8 -
 include/uapi/linux/kvm.h                      |   9 +-
 .../selftests/kvm/x86_64/xen_shinfo_test.c    |  59 ++-
 virt/kvm/pfncache.c                           | 382 ++++++++++--------
 13 files changed, 591 insertions(+), 363 deletions(-)


base-commit: 7455665a3521aa7b56245c0a2810f748adc5fdd4

Comments

Sean Christopherson Feb. 19, 2024, 10:06 p.m. UTC | #1 
On Thu, Feb 15, 2024, Paul Durrant wrote:
> David Woodhouse (1):
>   KVM: pfncache: rework __kvm_gpc_refresh() to fix locking issues
> 
> Paul Durrant (19):
>   KVM: pfncache: Add a map helper function
>   KVM: pfncache: remove unnecessary exports
>   KVM: x86/xen: mark guest pages dirty with the pfncache lock held
>   KVM: pfncache: add a mark-dirty helper
>   KVM: pfncache: remove KVM_GUEST_USES_PFN usage
>   KVM: pfncache: stop open-coding offset_in_page()
>   KVM: pfncache: include page offset in uhva and use it consistently
>   KVM: pfncache: allow a cache to be activated with a fixed (userspace)
>     HVA
>   KVM: x86/xen: separate initialization of shared_info cache and content
>   KVM: x86/xen: re-initialize shared_info if guest (32/64-bit) mode is
>     set
>   KVM: x86/xen: allow shared_info to be mapped by fixed HVA
>   KVM: x86/xen: allow vcpu_info to be mapped by fixed HVA
>   KVM: selftests: map Xen's shared_info page using HVA rather than GFN
>   KVM: selftests: re-map Xen's vcpu_info using HVA rather than GPA
>   KVM: x86/xen: advertize the KVM_XEN_HVM_CONFIG_SHARED_INFO_HVA
>     capability
>   KVM: x86/xen: split up kvm_xen_set_evtchn_fast()
>   KVM: x86/xen: don't block on pfncache locks in
>     kvm_xen_set_evtchn_fast()
>   KVM: pfncache: check the need for invalidation under read lock first
>   KVM: x86/xen: allow vcpu_info content to be 'safely' copied
> 
> Sean Christopherson (1):
>   KVM: s390: Refactor kvm_is_error_gpa() into kvm_is_gpa_in_memslot()
> 
>  Documentation/virt/kvm/api.rst                |  53 ++-
>  arch/s390/kvm/diag.c                          |   2 +-
>  arch/s390/kvm/gaccess.c                       |  14 +-
>  arch/s390/kvm/kvm-s390.c                      |   4 +-
>  arch/s390/kvm/priv.c                          |   4 +-
>  arch/s390/kvm/sigp.c                          |   2 +-
>  arch/x86/kvm/x86.c                            |   7 +-
>  arch/x86/kvm/xen.c                            | 361 +++++++++++------
>  include/linux/kvm_host.h                      |  49 ++-
>  include/linux/kvm_types.h                     |   8 -
>  include/uapi/linux/kvm.h                      |   9 +-
>  .../selftests/kvm/x86_64/xen_shinfo_test.c    |  59 ++-
>  virt/kvm/pfncache.c                           | 382 ++++++++++--------
>  13 files changed, 591 insertions(+), 363 deletions(-)

Except for the read_trylock() patch, just a few nits that I can fixup when
applying, though I'll defeinitely want your eyeballs on the end result as they
tweaks aren't _that_ trivial.

Running tests now, if all goes well I'll push to kvm-x86 within the hour.
Paul Durrant Feb. 20, 2024, 9:14 a.m. UTC | #2 
On 19/02/2024 22:06, Sean Christopherson wrote:
> On Thu, Feb 15, 2024, Paul Durrant wrote:
>> David Woodhouse (1):
>>    KVM: pfncache: rework __kvm_gpc_refresh() to fix locking issues
>>
>> Paul Durrant (19):
>>    KVM: pfncache: Add a map helper function
>>    KVM: pfncache: remove unnecessary exports
>>    KVM: x86/xen: mark guest pages dirty with the pfncache lock held
>>    KVM: pfncache: add a mark-dirty helper
>>    KVM: pfncache: remove KVM_GUEST_USES_PFN usage
>>    KVM: pfncache: stop open-coding offset_in_page()
>>    KVM: pfncache: include page offset in uhva and use it consistently
>>    KVM: pfncache: allow a cache to be activated with a fixed (userspace)
>>      HVA
>>    KVM: x86/xen: separate initialization of shared_info cache and content
>>    KVM: x86/xen: re-initialize shared_info if guest (32/64-bit) mode is
>>      set
>>    KVM: x86/xen: allow shared_info to be mapped by fixed HVA
>>    KVM: x86/xen: allow vcpu_info to be mapped by fixed HVA
>>    KVM: selftests: map Xen's shared_info page using HVA rather than GFN
>>    KVM: selftests: re-map Xen's vcpu_info using HVA rather than GPA
>>    KVM: x86/xen: advertize the KVM_XEN_HVM_CONFIG_SHARED_INFO_HVA
>>      capability
>>    KVM: x86/xen: split up kvm_xen_set_evtchn_fast()
>>    KVM: x86/xen: don't block on pfncache locks in
>>      kvm_xen_set_evtchn_fast()
>>    KVM: pfncache: check the need for invalidation under read lock first
>>    KVM: x86/xen: allow vcpu_info content to be 'safely' copied
>>
>> Sean Christopherson (1):
>>    KVM: s390: Refactor kvm_is_error_gpa() into kvm_is_gpa_in_memslot()
>>
>>   Documentation/virt/kvm/api.rst                |  53 ++-
>>   arch/s390/kvm/diag.c                          |   2 +-
>>   arch/s390/kvm/gaccess.c                       |  14 +-
>>   arch/s390/kvm/kvm-s390.c                      |   4 +-
>>   arch/s390/kvm/priv.c                          |   4 +-
>>   arch/s390/kvm/sigp.c                          |   2 +-
>>   arch/x86/kvm/x86.c                            |   7 +-
>>   arch/x86/kvm/xen.c                            | 361 +++++++++++------
>>   include/linux/kvm_host.h                      |  49 ++-
>>   include/linux/kvm_types.h                     |   8 -
>>   include/uapi/linux/kvm.h                      |   9 +-
>>   .../selftests/kvm/x86_64/xen_shinfo_test.c    |  59 ++-
>>   virt/kvm/pfncache.c                           | 382 ++++++++++--------
>>   13 files changed, 591 insertions(+), 363 deletions(-)
> 
> Except for the read_trylock() patch, just a few nits that I can fixup when
> applying, though I'll defeinitely want your eyeballs on the end result as they
> tweaks aren't _that_ trivial.
> 
> Running tests now, if all goes well I'll push to kvm-x86 within the hour.

Oh, I read this last and you already made the changes :-) I'll check 
kvm-x86. Thanks.
Paul Durrant Feb. 20, 2024, 10:53 a.m. UTC | #3 
On 20/02/2024 09:14, Paul Durrant wrote:
> On 19/02/2024 22:06, Sean Christopherson wrote:
>> On Thu, Feb 15, 2024, Paul Durrant wrote:
>>> David Woodhouse (1):
>>>    KVM: pfncache: rework __kvm_gpc_refresh() to fix locking issues
>>>
>>> Paul Durrant (19):
>>>    KVM: pfncache: Add a map helper function
>>>    KVM: pfncache: remove unnecessary exports
>>>    KVM: x86/xen: mark guest pages dirty with the pfncache lock held
>>>    KVM: pfncache: add a mark-dirty helper
>>>    KVM: pfncache: remove KVM_GUEST_USES_PFN usage
>>>    KVM: pfncache: stop open-coding offset_in_page()
>>>    KVM: pfncache: include page offset in uhva and use it consistently
>>>    KVM: pfncache: allow a cache to be activated with a fixed (userspace)
>>>      HVA
>>>    KVM: x86/xen: separate initialization of shared_info cache and 
>>> content
>>>    KVM: x86/xen: re-initialize shared_info if guest (32/64-bit) mode is
>>>      set
>>>    KVM: x86/xen: allow shared_info to be mapped by fixed HVA
>>>    KVM: x86/xen: allow vcpu_info to be mapped by fixed HVA
>>>    KVM: selftests: map Xen's shared_info page using HVA rather than GFN
>>>    KVM: selftests: re-map Xen's vcpu_info using HVA rather than GPA
>>>    KVM: x86/xen: advertize the KVM_XEN_HVM_CONFIG_SHARED_INFO_HVA
>>>      capability
>>>    KVM: x86/xen: split up kvm_xen_set_evtchn_fast()
>>>    KVM: x86/xen: don't block on pfncache locks in
>>>      kvm_xen_set_evtchn_fast()
>>>    KVM: pfncache: check the need for invalidation under read lock first
>>>    KVM: x86/xen: allow vcpu_info content to be 'safely' copied
>>>
>>> Sean Christopherson (1):
>>>    KVM: s390: Refactor kvm_is_error_gpa() into kvm_is_gpa_in_memslot()
>>>
>>>   Documentation/virt/kvm/api.rst                |  53 ++-
>>>   arch/s390/kvm/diag.c                          |   2 +-
>>>   arch/s390/kvm/gaccess.c                       |  14 +-
>>>   arch/s390/kvm/kvm-s390.c                      |   4 +-
>>>   arch/s390/kvm/priv.c                          |   4 +-
>>>   arch/s390/kvm/sigp.c                          |   2 +-
>>>   arch/x86/kvm/x86.c                            |   7 +-
>>>   arch/x86/kvm/xen.c                            | 361 +++++++++++------
>>>   include/linux/kvm_host.h                      |  49 ++-
>>>   include/linux/kvm_types.h                     |   8 -
>>>   include/uapi/linux/kvm.h                      |   9 +-
>>>   .../selftests/kvm/x86_64/xen_shinfo_test.c    |  59 ++-
>>>   virt/kvm/pfncache.c                           | 382 ++++++++++--------
>>>   13 files changed, 591 insertions(+), 363 deletions(-)
>>
>> Except for the read_trylock() patch, just a few nits that I can fixup 
>> when
>> applying, though I'll defeinitely want your eyeballs on the end result 
>> as they
>> tweaks aren't _that_ trivial.
>>
>> Running tests now, if all goes well I'll push to kvm-x86 within the hour.
> 
> Oh, I read this last and you already made the changes :-) I'll check 
> kvm-x86. Thanks.
> 

I checked the patches you amended. All LGTM and my tests are fine too.
Sean Christopherson Feb. 20, 2024, 3:55 p.m. UTC | #4 
On Thu, 15 Feb 2024 15:28:55 +0000, Paul Durrant wrote:
> From: Paul Durrant <pdurrant@amazon.com>
> 
> This series contains a new patch from Sean added since v12 [1]:
> 
> * KVM: s390: Refactor kvm_is_error_gpa() into kvm_is_gpa_in_memslot()
> 
> This frees up the function name kvm_is_error_gpa() such that it can then be
> re-defined in:
> 
> [...]

*sigh*

I forgot to hit "send" on this yesterday.  But lucky for me, that worked out in
my favor as I needed to rebase on top of kvm/kvm-uapi to avoid pointless conflicts
in the uapi headeres.

So....

Applied to kvm-x86 xen, minus 18 and 19 (trylock stuff) and 21 (locking cleanup
that we're doing elsewhere).

Paul and David, please take (another) look at the end result to make sure you don't
object to any of my tweaks and that I didn't botch anything.

s390 folks, I'm applying/pushing now to get it into -next asap, but I'll make
sure to get acks/reviews on patch 08/21 before I do anything else with this
branch/series.

Thanks!

[01/21] KVM: pfncache: Add a map helper function
        https://github.com/kvm-x86/linux/commit/f39b80e3ff12
[02/21] KVM: pfncache: remove unnecessary exports
        https://github.com/kvm-x86/linux/commit/41496fffc0e1
[03/21] KVM: x86/xen: mark guest pages dirty with the pfncache lock held
        https://github.com/kvm-x86/linux/commit/4438355ec6e1
[04/21] KVM: pfncache: add a mark-dirty helper
        https://github.com/kvm-x86/linux/commit/78b74638eb6d
[05/21] KVM: pfncache: remove KVM_GUEST_USES_PFN usage
        https://github.com/kvm-x86/linux/commit/a4bff3df5147
[06/21] KVM: pfncache: stop open-coding offset_in_page()
        https://github.com/kvm-x86/linux/commit/53e63e953e14
[07/21] KVM: pfncache: include page offset in uhva and use it consistently
        https://github.com/kvm-x86/linux/commit/406c10962a4c
[08/21] KVM: s390: Refactor kvm_is_error_gpa() into kvm_is_gpa_in_memslot()
        https://github.com/kvm-x86/linux/commit/9e7325acb3dc
[09/21] KVM: pfncache: allow a cache to be activated with a fixed (userspace) HVA
        https://github.com/kvm-x86/linux/commit/721f5b0dda78
[10/21] KVM: x86/xen: separate initialization of shared_info cache and content
        https://github.com/kvm-x86/linux/commit/c01c55a34f28
[11/21] KVM: x86/xen: re-initialize shared_info if guest (32/64-bit) mode is set
        https://github.com/kvm-x86/linux/commit/21b99e4d6db6
[12/21] KVM: x86/xen: allow shared_info to be mapped by fixed HVA
        https://github.com/kvm-x86/linux/commit/10dcbfc46724
[13/21] KVM: x86/xen: allow vcpu_info to be mapped by fixed HVA
        https://github.com/kvm-x86/linux/commit/16877dd45f98
[14/21] KVM: selftests: map Xen's shared_info page using HVA rather than GFN
        https://github.com/kvm-x86/linux/commit/95c27ed8619b
[15/21] KVM: selftests: re-map Xen's vcpu_info using HVA rather than GPA
        https://github.com/kvm-x86/linux/commit/5359bf19a3f0
[16/21] KVM: x86/xen: advertize the KVM_XEN_HVM_CONFIG_SHARED_INFO_HVA capability
        https://github.com/kvm-x86/linux/commit/49668ce7e1ae
[17/21] KVM: x86/xen: split up kvm_xen_set_evtchn_fast()
        (not applied)
[18/21] KVM: x86/xen: don't block on pfncache locks in kvm_xen_set_evtchn_fast()
        (not applied)
[19/21] KVM: pfncache: check the need for invalidation under read lock first
        https://github.com/kvm-x86/linux/commit/21dadfcd665e
[20/21] KVM: x86/xen: allow vcpu_info content to be 'safely' copied
        https://github.com/kvm-x86/linux/commit/dadeabc3b6fa
[21/21] KVM: pfncache: rework __kvm_gpc_refresh() to fix locking issues
        (not applied)

--
https://github.com/kvm-x86/linux/tree/next
Paul Durrant Feb. 20, 2024, 4:03 p.m. UTC | #5 
On 20/02/2024 15:55, Sean Christopherson wrote:
> On Thu, 15 Feb 2024 15:28:55 +0000, Paul Durrant wrote:
>> From: Paul Durrant <pdurrant@amazon.com>
>>
>> This series contains a new patch from Sean added since v12 [1]:
>>
>> * KVM: s390: Refactor kvm_is_error_gpa() into kvm_is_gpa_in_memslot()
>>
>> This frees up the function name kvm_is_error_gpa() such that it can then be
>> re-defined in:
>>
>> [...]
> 
> *sigh*
> 
> I forgot to hit "send" on this yesterday.  But lucky for me, that worked out in
> my favor as I needed to rebase on top of kvm/kvm-uapi to avoid pointless conflicts
> in the uapi headeres.
> 
> So....
> 
> Applied to kvm-x86 xen, minus 18 and 19 (trylock stuff) and 21 (locking cleanup
> that we're doing elsewhere).
> 

Looks like you meant 17 & 18?

> Paul and David, please take (another) look at the end result to make sure you don't
> object to any of my tweaks and that I didn't botch anything.
> 

What was the issue with 17? It was reasonable clean-up and I'd like to 
keep it even without 18 being applied (and I totally understand your 
reasons for that).

> s390 folks, I'm applying/pushing now to get it into -next asap, but I'll make
> sure to get acks/reviews on patch 08/21 before I do anything else with this
> branch/series.
> 
> Thanks!
> 
> [01/21] KVM: pfncache: Add a map helper function
>          https://github.com/kvm-x86/linux/commit/f39b80e3ff12
> [02/21] KVM: pfncache: remove unnecessary exports
>          https://github.com/kvm-x86/linux/commit/41496fffc0e1
> [03/21] KVM: x86/xen: mark guest pages dirty with the pfncache lock held
>          https://github.com/kvm-x86/linux/commit/4438355ec6e1
> [04/21] KVM: pfncache: add a mark-dirty helper
>          https://github.com/kvm-x86/linux/commit/78b74638eb6d
> [05/21] KVM: pfncache: remove KVM_GUEST_USES_PFN usage
>          https://github.com/kvm-x86/linux/commit/a4bff3df5147
> [06/21] KVM: pfncache: stop open-coding offset_in_page()
>          https://github.com/kvm-x86/linux/commit/53e63e953e14
> [07/21] KVM: pfncache: include page offset in uhva and use it consistently
>          https://github.com/kvm-x86/linux/commit/406c10962a4c
> [08/21] KVM: s390: Refactor kvm_is_error_gpa() into kvm_is_gpa_in_memslot()
>          https://github.com/kvm-x86/linux/commit/9e7325acb3dc
> [09/21] KVM: pfncache: allow a cache to be activated with a fixed (userspace) HVA
>          https://github.com/kvm-x86/linux/commit/721f5b0dda78
> [10/21] KVM: x86/xen: separate initialization of shared_info cache and content
>          https://github.com/kvm-x86/linux/commit/c01c55a34f28
> [11/21] KVM: x86/xen: re-initialize shared_info if guest (32/64-bit) mode is set
>          https://github.com/kvm-x86/linux/commit/21b99e4d6db6
> [12/21] KVM: x86/xen: allow shared_info to be mapped by fixed HVA
>          https://github.com/kvm-x86/linux/commit/10dcbfc46724
> [13/21] KVM: x86/xen: allow vcpu_info to be mapped by fixed HVA
>          https://github.com/kvm-x86/linux/commit/16877dd45f98
> [14/21] KVM: selftests: map Xen's shared_info page using HVA rather than GFN
>          https://github.com/kvm-x86/linux/commit/95c27ed8619b
> [15/21] KVM: selftests: re-map Xen's vcpu_info using HVA rather than GPA
>          https://github.com/kvm-x86/linux/commit/5359bf19a3f0
> [16/21] KVM: x86/xen: advertize the KVM_XEN_HVM_CONFIG_SHARED_INFO_HVA capability
>          https://github.com/kvm-x86/linux/commit/49668ce7e1ae
> [17/21] KVM: x86/xen: split up kvm_xen_set_evtchn_fast()
>          (not applied)
> [18/21] KVM: x86/xen: don't block on pfncache locks in kvm_xen_set_evtchn_fast()
>          (not applied)
> [19/21] KVM: pfncache: check the need for invalidation under read lock first
>          https://github.com/kvm-x86/linux/commit/21dadfcd665e
> [20/21] KVM: x86/xen: allow vcpu_info content to be 'safely' copied
>          https://github.com/kvm-x86/linux/commit/dadeabc3b6fa
> [21/21] KVM: pfncache: rework __kvm_gpc_refresh() to fix locking issues
>          (not applied)
> 
> --
> https://github.com/kvm-x86/linux/tree/next
Sean Christopherson Feb. 20, 2024, 4:15 p.m. UTC | #6 
On Tue, Feb 20, 2024, Paul Durrant wrote:
> On 20/02/2024 15:55, Sean Christopherson wrote:
> > On Thu, 15 Feb 2024 15:28:55 +0000, Paul Durrant wrote:
> > > From: Paul Durrant <pdurrant@amazon.com>
> > > 
> > > This series contains a new patch from Sean added since v12 [1]:
> > > 
> > > * KVM: s390: Refactor kvm_is_error_gpa() into kvm_is_gpa_in_memslot()
> > > 
> > > This frees up the function name kvm_is_error_gpa() such that it can then be
> > > re-defined in:
> > > 
> > > [...]
> > 
> > *sigh*
> > 
> > I forgot to hit "send" on this yesterday.  But lucky for me, that worked out in
> > my favor as I needed to rebase on top of kvm/kvm-uapi to avoid pointless conflicts
> > in the uapi headeres.
> > 
> > So....
> > 
> > Applied to kvm-x86 xen, minus 18 and 19 (trylock stuff) and 21 (locking cleanup
> > that we're doing elsewhere).
> > 
> 
> Looks like you meant 17 & 18?

Doh, yes.

> > Paul and David, please take (another) look at the end result to make sure you don't
> > object to any of my tweaks and that I didn't botch anything.
> > 
> 
> What was the issue with 17? It was reasonable clean-up and I'd like to keep
> it even without 18 being applied (and I totally understand your reasons for
> that).

I omitted it purely to avoid creating an unnecessary dependency for the trylock
patch.  That way the trylock patch (or whatever it morphs into) can be applied on
any branch (along with the cleanup), i.e. doesn't need to be taken through kvm-x86/xen.
David Woodhouse Feb. 20, 2024, 4:21 p.m. UTC | #7 
On 20 February 2024 17:15:06 CET, Sean Christopherson <seanjc@google.com> wrote:
>On Tue, Feb 20, 2024, Paul Durrant wrote:
>> On 20/02/2024 15:55, Sean Christopherson wrote:
>> > On Thu, 15 Feb 2024 15:28:55 +0000, Paul Durrant wrote:
>> > > From: Paul Durrant <pdurrant@amazon.com>
>> > > 
>> > > This series contains a new patch from Sean added since v12 [1]:
>> > > 
>> > > * KVM: s390: Refactor kvm_is_error_gpa() into kvm_is_gpa_in_memslot()
>> > > 
>> > > This frees up the function name kvm_is_error_gpa() such that it can then be
>> > > re-defined in:
>> > > 
>> > > [...]
>> > 
>> > *sigh*
>> > 
>> > I forgot to hit "send" on this yesterday.  But lucky for me, that worked out in
>> > my favor as I needed to rebase on top of kvm/kvm-uapi to avoid pointless conflicts
>> > in the uapi headeres.
>> > 
>> > So....
>> > 
>> > Applied to kvm-x86 xen, minus 18 and 19 (trylock stuff) and 21 (locking cleanup
>> > that we're doing elsewhere).
>> > 
>> 
>> Looks like you meant 17 & 18?
>
>Doh, yes.
>
>> > Paul and David, please take (another) look at the end result to make sure you don't
>> > object to any of my tweaks and that I didn't botch anything.
>> > 
>> 
>> What was the issue with 17? It was reasonable clean-up and I'd like to keep
>> it even without 18 being applied (and I totally understand your reasons for
>> that).
>
>I omitted it purely to avoid creating an unnecessary dependency for the trylock
>patch.  That way the trylock patch (or whatever it morphs into) can be applied on
>any branch (along with the cleanup), i.e. doesn't need to be taken through kvm-x86/xen.

What about if (in_atomic() && read_trylock()) return -EAGAIN; else read_lock();

That way we don't have any even theoretical fairness issues because the trylock can fail just *once* which kicks us to the slow path and that'll take the lock normally now.

The condition might not actually be in_atomic() but I'm not working this week and you get the idea.
Paul Durrant Feb. 20, 2024, 5:07 p.m. UTC | #8 
On 20/02/2024 16:15, Sean Christopherson wrote:
> On Tue, Feb 20, 2024, Paul Durrant wrote:
>> On 20/02/2024 15:55, Sean Christopherson wrote:
>>> On Thu, 15 Feb 2024 15:28:55 +0000, Paul Durrant wrote:
>>>> From: Paul Durrant <pdurrant@amazon.com>
>>>>
>>>> This series contains a new patch from Sean added since v12 [1]:
>>>>
>>>> * KVM: s390: Refactor kvm_is_error_gpa() into kvm_is_gpa_in_memslot()
>>>>
>>>> This frees up the function name kvm_is_error_gpa() such that it can then be
>>>> re-defined in:
>>>>
>>>> [...]
>>>
>>> *sigh*
>>>
>>> I forgot to hit "send" on this yesterday.  But lucky for me, that worked out in
>>> my favor as I needed to rebase on top of kvm/kvm-uapi to avoid pointless conflicts
>>> in the uapi headeres.
>>>
>>> So....
>>>
>>> Applied to kvm-x86 xen, minus 18 and 19 (trylock stuff) and 21 (locking cleanup
>>> that we're doing elsewhere).
>>>
>>
>> Looks like you meant 17 & 18?
> 
> Doh, yes.
> 
>>> Paul and David, please take (another) look at the end result to make sure you don't
>>> object to any of my tweaks and that I didn't botch anything.
>>>
>>
>> What was the issue with 17? It was reasonable clean-up and I'd like to keep
>> it even without 18 being applied (and I totally understand your reasons for
>> that).
> 
> I omitted it purely to avoid creating an unnecessary dependency for the trylock
> patch.  That way the trylock patch (or whatever it morphs into) can be applied on
> any branch (along with the cleanup), i.e. doesn't need to be taken through kvm-x86/xen.

Ok, personally I don't see the dependency being an issue. I suspect it 
will be a while before we decide what to do about the locking issue... 
particularly since David is out this week, as he says.