>> >>> + 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 -- To unsubscribe from this list: send the line "unsubscribe linux-s390" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html