On 04.02.20 11:57, Cornelia Huck wrote:
>>
>> /* Generic bits for GMAP notification on DAT table entry changes. */
>> @@ -61,6 +62,7 @@ struct gmap {
>> spinlock_t shadow_lock;
>> struct gmap *parent;
>> unsigned long orig_asce;
>> + unsigned long se_handle;
>
> This is a deviation from the "protected virtualization" naming scheme
> used in the previous patches, but I understand that the naming of this
> whole feature is still in flux :) (Would still be nice to have it
> consistent, though.)
>
> However, I think I'd prefer something named based on protected
> virtualization: the se_* stuff here just tends to make me think of
> SELinux...
I will just use guest_handle as this guests assigned to and from such:
arch/s390/include/asm/gmap.h: unsigned long se_handle;
arch/s390/include/asm/uv.h: .guest_handle = gmap->se_handle,
arch/s390/kvm/pv.c: WRITE_ONCE(kvm->arch.gmap->se_handle, 0);
arch/s390/kvm/pv.c: kvm->arch.gmap->se_handle = uvcb.guest_handle;
arch/s390/mm/fault.c: .guest_handle = gmap->se_handle,
>
>> int edat_level;
>> bool removed;
>> bool initialized;
>> diff --git a/arch/s390/include/asm/mmu.h b/arch/s390/include/asm/mmu.h
>> index bcfb6371086f..984026cb3608 100644
>> --- a/arch/s390/include/asm/mmu.h
>> +++ b/arch/s390/include/asm/mmu.h
>> @@ -16,6 +16,8 @@ typedef struct {
>> unsigned long asce;
>> unsigned long asce_limit;
>> unsigned long vdso_base;
>> + /* The mmu context belongs to a secure guest. */
>> + atomic_t is_se;
>
> Here as well.
I will use is "is_protected"
>
>> /*
>> * The following bitfields need a down_write on the mm
>> * semaphore when they are written to. As they are only
>
> (...)
>
>> @@ -520,6 +521,15 @@ static inline int mm_has_pgste(struct mm_struct *mm)
>> return 0;
>> }
>>
>> +static inline int mm_is_se(struct mm_struct *mm)
>> +{
>> +#ifdef CONFIG_KVM_S390_PROTECTED_VIRTUALIZATION_HOST
>> + if (unlikely(atomic_read(&mm->context.is_se)))
>> + return 1;
>> +#endif
>> + return 0;
>> +}
>
> I'm wondering how big of an overhead we actually have because we need
> to check the flag here if the feature is enabled. We have an extra
> check in a few functions anyway, even if protected virt is not
> configured in. Given that distributions would likely want to enable the
> feature in their kernels, I'm currently tending towards dropping the
> extra config option.
Makes sense. I will wait a bit more but if you do not disagree I will remove
the extra host config and bind things to CONFIG_KVM or CONFIG_PGSTE depending
on context
>
>> +
>> static inline int mm_alloc_pgste(struct mm_struct *mm)
>> {
>> #ifdef CONFIG_PGSTE
>
> (...)
>
>> diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c
>> index f7778493e829..136c60a8e3ca 100644
>> --- a/arch/s390/kernel/uv.c
>> +++ b/arch/s390/kernel/uv.c
>> @@ -9,6 +9,8 @@
>> #include <linux/sizes.h>
>> #include <linux/bitmap.h>
>> #include <linux/memblock.h>
>> +#include <linux/pagemap.h>
>> +#include <linux/swap.h>
>> #include <asm/facility.h>
>> #include <asm/sections.h>
>> #include <asm/uv.h>
>> @@ -98,4 +100,172 @@ void adjust_to_uv_max(unsigned long *vmax)
>> if (prot_virt_host && *vmax > uv_info.max_sec_stor_addr)
>> *vmax = uv_info.max_sec_stor_addr;
>> }
>> +
>> +static int __uv_pin_shared(unsigned long paddr)
>> +{
>> + struct uv_cb_cfs uvcb = {
>> + .header.cmd = UVC_CMD_PIN_PAGE_SHARED,
>> + .header.len = sizeof(uvcb),
>> + .paddr = paddr,
>> + };
>> +
>> + if (uv_call(0, (u64)&uvcb))
>> + return -EINVAL;
>> + return 0;
>
> I guess this call won't set an error rc in the control block, if it
> does not set a condition code != 0?
Right. RC is only set for CC!=0
