Re: [RFC/PATCH v2 01/22] s390/mm: make gmap_protect_range more modular

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 22.01.2018 13:50, David Hildenbrand wrote:
> 
>>>
>>>> +	if (!pmdp || pmd_none(*pmdp)) {
>>>> +		spin_unlock(&gmap->guest_table_lock);
>>>> +		return NULL;
>>>> +	}
>>>> +	/*
>>>> +	 * For plain 4k guests that do not run under the vsie it
>>>> +	 * suffices to take the pte lock later on. Thus we can unlock
>>>> +	 * the guest_table_lock here.
>>>> +	 */
>>>
>>> As discussed, the gmap_is_shadow() check is not needed. The comment
>>> should be something like
>>
>> IFF we'll never use this function to walk shadow tables, then you are
>> right. We can make it a policy and throw in a BUG_ON.
> 
> Right. We never protect anything on a shadow gmap. We only mirror the
> access tights requested by the guest (which are then valid in the host).

For now I'll introduce a comment, a BUG_ON and get rid of the check.

> 
>>
>> [...]
>>>> +static int gmap_protect_pte(struct gmap *gmap, unsigned long gaddr,
>>>> +			    pmd_t *pmdp, int prot, unsigned long bits)
>>>> +{
>>>> +	int rc;
>>>> +	pte_t *ptep;
>>>> +	spinlock_t *ptl = NULL;
>>>> +
>>>> +	/* We have no upper segment, let's go back and fix this up. */
>>>> +	if (pmd_val(*pmdp) & _SEGMENT_ENTRY_INVALID)
>>>> +		return -EAGAIN;
>>>
>>> This is essentially pmd_none(*pmdp), which you already verified in
>>> gmap_pmd_op_walk().
>>
>> Well, not really pmd_none is entry == ENTRY_EMPTY (only I bit set) not
>> entry & I.
>> Is there a path where we have an I bit on a pmd entry which has a valid pto?
>>
> 
> Thing idte only sets the invalid bit. But can this check than go into
> gmap_pmd_op_walk? (replacing pmd_none() ?)

I'll have to think about that, but quite possibly yes.
The problem comes from Martin's wish to properly handle prot_none entries.



Attachment: signature.asc
Description: OpenPGP digital signature


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux