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

[David Hildenbrand <david@redhat.com>]

>>
>>> +	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).

> 
> [...]
>>> +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() ?)

-- 

Thanks,

David / dhildenb