On 12.07.2018 10:09, frankja wrote:
> On Thu, 12 Jul 2018 09:23:01 +0200
> David Hildenbrand <david@xxxxxxxxxx> wrote:
>>
>>> #include <linux/kernel.h>
>>> @@ -589,8 +591,8 @@ int __gmap_link(struct gmap *gmap, unsigned
>>> long gaddr, unsigned long vmaddr) return -EFAULT;
>>> pmd = pmd_offset(pud, vmaddr);
>>> VM_BUG_ON(pmd_none(*pmd));
>>> - /* large pmds cannot yet be handled */
>>> - if (pmd_large(*pmd))
>>> + /* Are we allowed to use huge pages? */
>>> + if (pmd_large(*pmd) && !gmap->mm->context.uses_gmap_hpage)
>>> return -EFAULT;
>>> /* Link gmap segment table entry location to page table. */
>>> rc = radix_tree_preload(GFP_KERNEL);
>>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>>> index b6270a3b38e9..b955b986b341 100644
>>> --- a/include/uapi/linux/kvm.h
>>> +++ b/include/uapi/linux/kvm.h
>>> @@ -949,6 +949,7 @@ struct kvm_ppc_resize_hpt {
>>> #define KVM_CAP_GET_MSR_FEATURES 153
>>> #define KVM_CAP_HYPERV_EVENTFD 154
>>> #define KVM_CAP_HYPERV_TLBFLUSH 155
>>> +#define KVM_CAP_S390_HPAGE_1M 156
>>>
>>> #ifdef KVM_CAP_IRQ_ROUTING
>>>
>>>
>>
>> I was wondering if we should add some safety net for gmap shadows when
>> hitting a huge pmd. Or when trying to create a shadow for a gmap with
>> huge pmds enabled (add a check in gmap_shadow()). So the GMAP kernel
>> interface remains consistent.
>
> For now we don't have the sief2 so we don't end up in gmap_shadow()
> anyway. Huge l3 in small l2 is currently supported via the fake tables.
>
> So what do you want to keep consistent?
gmap is just an internal kernel interface and KVM is the only user right
now. The logic that gmap shadow does not work with hpage gmaps is right
now buried in KVM, not in the GMAP. That's why I am asking if it makes
sense to also have safety nets in the gmap shadow code.
If you added the other safety net I suggested (PMD protection with
SHADOW notifier bit set -> -EINVAL), we are maybe already on the safe
side that if gmap_shadow() would be used. We would maybe fail in a nice
fashion. But I didn't check all call paths. Prohibiting gmap_shadow()
when huge pages are allowed in the GMAP right from the start would be
the big hammer.
--
Thanks,
David / dhildenb
