[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 4/7] s390x/mmu: Inject PGM_ADDRESSING on boguous table add
From: |
Richard Henderson |
Subject: |
Re: [PATCH v2 4/7] s390x/mmu: Inject PGM_ADDRESSING on boguous table addresses |
Date: |
Wed, 25 Sep 2019 12:25:10 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 |
On 9/25/19 5:52 AM, David Hildenbrand wrote:
> +static inline int read_table_entry(hwaddr gaddr, uint64_t *entry)
> +{
> + /*
> + * According to the PoP, these table addresses are "unpredictably real
> + * or absolute". Also, "it is unpredictable whether the address wraps
> + * or an addressing exception is recognized".
> + *
> + * We treat them as absolute addresses and don't wrap them.
> + */
> + if (unlikely(address_space_read(&address_space_memory, gaddr,
> + MEMTXATTRS_UNSPECIFIED, (uint8_t *)entry, sizeof(*entry)) !=
> + MEMTX_OK)) {
> + return -EFAULT;
> + }
> + *entry = be64_to_cpu(*entry);
> + return 0;
> +}
Maybe I've been away from the kernel too long, but I don't find returning
-EFAULT helpful. I would return true/false for success/failure so that...
> + if (read_table_entry(origin + offs, &pt_entry)) {
> + return PGM_ADDRESSING;
> + }
... this gets written
if (!read_table_entry(...)) {
return PGM_ADDRESSING;
}
This statement, to me, reads "If we did not read_table_entry, return an
addressing exception."
If you *really* want to return non-zero on failure, I would prefer returning
PGM_ADDRESSING instead of the out-of-context -EFAULT.
> - new_entry = ldq_phys(cs->as, origin + offs);
> + if (read_table_entry(origin + offs, &new_entry)) {
Do you really want to replace cs->as with address_space_memory?
r~
- Re: [PATCH v2 1/7] s390x/mmu: Drop debug logging from MMU code, (continued)
- [PATCH v2 2/7] s390x/mmu: Move DAT protection handling out of mmu_translate_asce(), David Hildenbrand, 2019/09/25
- [PATCH v2 3/7] s390x/mmu: Inject DAT exceptions from a single place, David Hildenbrand, 2019/09/25
- [PATCH v2 4/7] s390x/mmu: Inject PGM_ADDRESSING on boguous table addresses, David Hildenbrand, 2019/09/25
- [PATCH v2 5/7] s390x/mmu: Use TARGET_PAGE_MASK in mmu_translate_pte(), David Hildenbrand, 2019/09/25
- [PATCH v2 6/7] s390x/mmu: DAT table definition overhaul, David Hildenbrand, 2019/09/25