Re: [PATCH RFC 1/2] KVM: s390 vsie: use READ_ONCE to access some SCB fields

Cornelia Huck
2018-01-23T12:26:58Z

On Tue, 16 Jan 2018 18:15:25 +0100
David Hildenbrand <david@redhat.com> wrote:

> Another VCPU might try to modify the SCB while we are creating the
> shadow SCB. In general this is no problem - unless the compiler decides
> to not load values once, but e.g. twice.
>
> For us, this is only relevant when checking/working with such values.
> E.g. the prefix value, the mso, state of transactional execution and
> addresses of satellite blocks.
>
> E.g. if we blindly forwards values (e.g. general purpose registers or

s/forwards/forward/

> execution controls after masking), we don't care.
>
> Leaving unpin_blocks() untouched for now, will handle it separatly.
>
> The worst thing right now that I can see would be a missed prefix
> un/remap (mso, prefix, tx) or using wrong guest addresses. Nothing
> critical, but let's try to avoid unpredictable behavior.
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> arch/s390/kvm/vsie.c | 50 +++++++++++++++++++++++++++++++-------------------
> 1 file changed, 31 insertions(+), 19 deletions(-)
>
> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
> index 5d6ae0326d9e..79ea444a0534 100644
> --- a/arch/s390/kvm/vsie.c
> +++ b/arch/s390/kvm/vsie.c
> @@ -28,7 +28,11 @@ struct vsie_page {
> * the same offset as that in struct sie_page!
> */
> struct mcck_volatile_info mcck_info; /* 0x0200 */
> - /* the pinned originial scb */
> + /*
> + * The pinned originial scb. Be aware that other VCPUs can modify

s/originial/original/

(as that line is touched anyway)

> + * it while we read from it. Values that are used for conditions or
> + * are reused conditionally, should be accessed via READ_ONCE.
> + */
> struct kvm_s390_sie_block *scb_o; /* 0x0218 */
> /* the shadow gmap in use by the vsie_page */
> struct gmap *gmap; /* 0x0220 */

Acked-by: Cornelia Huck <cohuck@redhat.com>