* Re: [PATCH v1 09/10] powerpc/pseries/iommu: Make use of DDW even if it does not map the partition
From: Alexey Kardashevskiy @ 2020-09-04 1:00 UTC (permalink / raw)
To: Leonardo Bras, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Christophe Leroy, Joel Stanley,
Thiago Jung Bauermann, Ram Pai, Brian King,
Murilo Fossa Vicentini, David Dai
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <deb7163077326399b3a3170ecb777117f7f6737b.camel@gmail.com>
On 02/09/2020 16:11, Leonardo Bras wrote:
> On Mon, 2020-08-31 at 14:35 +1000, Alexey Kardashevskiy wrote:
>>
>> On 29/08/2020 04:36, Leonardo Bras wrote:
>>> On Mon, 2020-08-24 at 15:17 +1000, Alexey Kardashevskiy wrote:
>>>> On 18/08/2020 09:40, Leonardo Bras wrote:
>>>>> As of today, if the biggest DDW that can be created can't map the whole
>>>>> partition, it's creation is skipped and the default DMA window
>>>>> "ibm,dma-window" is used instead.
>>>>>
>>>>> DDW is 16x bigger than the default DMA window,
>>>>
>>>> 16x only under very specific circumstances which are
>>>> 1. phyp
>>>> 2. sriov
>>>> 3. device class in hmc (or what that priority number is in the lpar config).
>>>
>>> Yeah, missing details.
>>>
>>>>> having the same amount of
>>>>> pages, but increasing the page size to 64k.
>>>>> Besides larger DMA window,
>>>>
>>>> "Besides being larger"?
>>>
>>> You are right there.
>>>
>>>>> it performs better for allocations over 4k,
>>>>
>>>> Better how?
>>>
>>> I was thinking for allocations larger than (512 * 4k), since >2
>>> hypercalls are needed here, and for 64k pages would still be just 1
>>> hypercall up to (512 * 64k).
>>> But yeah, not the usual case anyway.
>>
>> Yup.
>>
>>
>>>>> so it would be nice to use it instead.
>>>>
>>>> I'd rather say something like:
>>>> ===
>>>> So far we assumed we can map the guest RAM 1:1 to the bus which worked
>>>> with a small number of devices. SRIOV changes it as the user can
>>>> configure hundreds VFs and since phyp preallocates TCEs and does not
>>>> allow IOMMU pages bigger than 64K, it has to limit the number of TCEs
>>>> per a PE to limit waste of physical pages.
>>>> ===
>>>
>>> I mixed this in my commit message, it looks like this:
>>>
>>> ===
>>> powerpc/pseries/iommu: Make use of DDW for indirect mapping
>>>
>>> So far it's assumed possible to map the guest RAM 1:1 to the bus, which
>>> works with a small number of devices. SRIOV changes it as the user can
>>> configure hundreds VFs and since phyp preallocates TCEs and does not
>>> allow IOMMU pages bigger than 64K, it has to limit the number of TCEs
>>> per a PE to limit waste of physical pages.
>>>
>>> As of today, if the assumed direct mapping is not possible, DDW
>>> creation is skipped and the default DMA window "ibm,dma-window" is used
>>> instead.
>>>
>>> The default DMA window uses 4k pages instead of 64k pages, and since
>>> the amount of pages is the same,
>>
>> Is the amount really the same? I thought you can prioritize some VFs
>> over others (== allocate different number of TCEs). Does it really
>> matter if it is the same?
>
> On a conversation with Travis Pizel, he explained how it's supposed to
> work, and I understood this:
>
> When a VF is created, it will be assigned a capacity, like 4%, 20%, and
> so on. The number of 'TCE entries' that are available to that partition
> are proportional to that capacity.
>
> If we use the default DMA window, the IOMMU pagesize/entry will be 4k,
> and if we use DDW, we will get 64k pagesize. As the number of entries
> will be the same (for the same capacity), the total space that can be
> addressed by the IOMMU will be 16 times bigger. This sometimes enable
> direct mapping, but sometimes it's still not enough.
Good to know. This is still an implementation detail, QEMU does not
allocate TCEs like this.
>
> On Travis words :
> "A low capacity VF, with less resources available, will certainly have
> less DMA window capability than a high capacity VF. But, an 8GB DMA
> window (with 64k pages) is still 16x larger than an 512MB window (with
> 4K pages).
> A high capacity VF - for example, one that Leonardo has in his scenario
> - will go from 8GB (using 4K pages) to 128GB (using 64K pages) - again,
> 16x larger - but it's obviously still possible to create a partition
> that exceeds 128GB of memory in size."
Right except the default dma window is not 8GB, it is <=2GB.
>
>>
>>
>>> making use of DDW instead of the
>>> default DMA window for indirect mapping will expand in 16x the amount
>>> of memory that can be mapped on DMA.
>>
>> Stop saying "16x", it is not guaranteed by anything :)
>>
>>
>>> The DDW created will be used for direct mapping by default. [...]
>>> ===
>>>
>>> What do you think?
>>>
>>>>> The DDW created will be used for direct mapping by default.
>>>>> If it's not available, indirect mapping will be used instead.
>>>>>
>>>>> For indirect mapping, it's necessary to update the iommu_table so
>>>>> iommu_alloc() can use the DDW created. For this,
>>>>> iommu_table_update_window() is called when everything else succeeds
>>>>> at enable_ddw().
>>>>>
>>>>> Removing the default DMA window for using DDW with indirect mapping
>>>>> is only allowed if there is no current IOMMU memory allocated in
>>>>> the iommu_table. enable_ddw() is aborted otherwise.
>>>>>
>>>>> As there will never have both direct and indirect mappings at the same
>>>>> time, the same property name can be used for the created DDW.
>>>>>
>>>>> So renaming
>>>>> define DIRECT64_PROPNAME "linux,direct64-ddr-window-info"
>>>>> to
>>>>> define DMA64_PROPNAME "linux,dma64-ddr-window-info"
>>>>> looks the right thing to do.
>>>>
>>>> I know I suggested this but this does not look so good anymore as I
>>>> suspect it breaks kexec (from older kernel to this one) so you either
>>>> need to check for both DT names or just keep the old one. Changing the
>>>> macro name is fine.
>>>>
>>>
>>> Yeah, having 'direct' in the name don't really makes sense if it's used
>>> for indirect mapping. I will just add this new define instead of
>>> replacing the old one, and check for both.
>>> Is that ok?
>>
>> No, having two of these does not seem right or useful. It is pseries
>> which does not use petitboot (relies on grub instead so until the target
>> kernel is started, there will be no ddw) so realistically we need this
>> property for kexec/kdump which uses the same kernel but different
>> initramdisk so for that purpose we need the same property name.
>>
>> But I can see myself annoyed when I try petitboot in the hacked pseries
>> qemu and things may crash :) On this basis I'd suggest keeping the name
>> and adding a comment next to it that it is not always "direct" anymore.
>>
>
> Keeping the same name should bring more problems than solve.
> If we have indirect mapping and kexec() to an older kernel, it will
> think direct mapping is enabled, and trying to use a DMA address
> without doing H_PUT_* first may cause a crash.
>
> I tested with a new property name, and it doesn't crash.
> As the property is not found, it does try to create a new DDW, which
> fails and it falls back to using the default DMA window.
> The device that need the IOMMU don't work well, but when iommu_map()
> fails, it doesn't try to use the DMA address as valid.
Right, as discussed on slack.
>
>>
>>>>> To make sure the property differentiates both cases, a new u32 for flags
>>>>> was added at the end of the property, where BIT(0) set means direct
>>>>> mapping.
>>>>>
>>>>> Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
>>>>> ---
>>>>> arch/powerpc/platforms/pseries/iommu.c | 108 +++++++++++++++++++------
>>>>> 1 file changed, 84 insertions(+), 24 deletions(-)
>>>>>
>>>>> diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
>>>>> index 3a1ef02ad9d5..9544e3c91ced 100644
>>>>> --- a/arch/powerpc/platforms/pseries/iommu.c
>>>>> +++ b/arch/powerpc/platforms/pseries/iommu.c
>>>>> @@ -350,8 +350,11 @@ struct dynamic_dma_window_prop {
>>>>> __be64 dma_base; /* address hi,lo */
>>>>> __be32 tce_shift; /* ilog2(tce_page_size) */
>>>>> __be32 window_shift; /* ilog2(tce_window_size) */
>>>>> + __be32 flags; /* DDW properties, see bellow */
>>>>> };
>>>>>
>>>>> +#define DDW_FLAGS_DIRECT 0x01
>>>>
>>>> This is set if ((1<<window_shift) >= ddw_memory_hotplug_max()), you
>>>> could simply check window_shift and drop the flags.
>>>>
>>>
>>> Yeah, it's better this way, I will revert this.
>>>
>>>>> +
>>>>> struct direct_window {
>>>>> struct device_node *device;
>>>>> const struct dynamic_dma_window_prop *prop;
>>>>> @@ -377,7 +380,7 @@ static LIST_HEAD(direct_window_list);
>>>>> static DEFINE_SPINLOCK(direct_window_list_lock);
>>>>> /* protects initializing window twice for same device */
>>>>> static DEFINE_MUTEX(direct_window_init_mutex);
>>>>> -#define DIRECT64_PROPNAME "linux,direct64-ddr-window-info"
>>>>> +#define DMA64_PROPNAME "linux,dma64-ddr-window-info"
>>>>>
>>>>> static int tce_clearrange_multi_pSeriesLP(unsigned long start_pfn,
>>>>> unsigned long num_pfn, const void *arg)
>>>>> @@ -836,7 +839,7 @@ static void remove_ddw(struct device_node *np, bool remove_prop)
>>>>> if (ret)
>>>>> return;
>>>>>
>>>>> - win = of_find_property(np, DIRECT64_PROPNAME, NULL);
>>>>> + win = of_find_property(np, DMA64_PROPNAME, NULL);
>>>>> if (!win)
>>>>> return;
>>>>>
>>>>> @@ -852,7 +855,7 @@ static void remove_ddw(struct device_node *np, bool remove_prop)
>>>>> np, ret);
>>>>> }
>>>>>
>>>>> -static bool find_existing_ddw(struct device_node *pdn, u64 *dma_addr)
>>>>> +static bool find_existing_ddw(struct device_node *pdn, u64 *dma_addr, bool *direct_mapping)
>>>>> {
>>>>> struct direct_window *window;
>>>>> const struct dynamic_dma_window_prop *direct64;
>>>>> @@ -864,6 +867,7 @@ static bool find_existing_ddw(struct device_node *pdn, u64 *dma_addr)
>>>>> if (window->device == pdn) {
>>>>> direct64 = window->prop;
>>>>> *dma_addr = be64_to_cpu(direct64->dma_base);
>>>>> + *direct_mapping = be32_to_cpu(direct64->flags) & DDW_FLAGS_DIRECT;
>>>>> found = true;
>>>>> break;
>>>>> }
>>>>> @@ -901,8 +905,8 @@ static int find_existing_ddw_windows(void)
>>>>> if (!firmware_has_feature(FW_FEATURE_LPAR))
>>>>> return 0;
>>>>>
>>>>> - for_each_node_with_property(pdn, DIRECT64_PROPNAME) {
>>>>> - direct64 = of_get_property(pdn, DIRECT64_PROPNAME, &len);
>>>>> + for_each_node_with_property(pdn, DMA64_PROPNAME) {
>>>>> + direct64 = of_get_property(pdn, DMA64_PROPNAME, &len);
>>>>> if (!direct64)
>>>>> continue;
>>>>>
>>>>> @@ -1124,7 +1128,8 @@ static void reset_dma_window(struct pci_dev *dev, struct device_node *par_dn)
>>>>> }
>>>>>
>>>>> static int ddw_property_create(struct property **ddw_win, const char *propname,
>>>>> - u32 liobn, u64 dma_addr, u32 page_shift, u32 window_shift)
>>>>> + u32 liobn, u64 dma_addr, u32 page_shift,
>>>>> + u32 window_shift, bool direct_mapping)
>>>>> {
>>>>> struct dynamic_dma_window_prop *ddwprop;
>>>>> struct property *win64;
>>>>> @@ -1144,6 +1149,36 @@ static int ddw_property_create(struct property **ddw_win, const char *propname,
>>>>> ddwprop->dma_base = cpu_to_be64(dma_addr);
>>>>> ddwprop->tce_shift = cpu_to_be32(page_shift);
>>>>> ddwprop->window_shift = cpu_to_be32(window_shift);
>>>>> + if (direct_mapping)
>>>>> + ddwprop->flags = cpu_to_be32(DDW_FLAGS_DIRECT);
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +static int iommu_table_update_window(struct iommu_table **tbl, int nid, unsigned long liobn,
>>>>> + unsigned long win_addr, unsigned long page_shift,
>>>>> + unsigned long window_size)
>>>>
>>>> Rather strange helper imho. I'd extract the most of
>>>> iommu_table_setparms_lpar() into iommu_table_setparms() (except
>>>> of_parse_dma_window) and call new helper from where you call
>>>> iommu_table_update_window; and do
>>>> iommu_pseries_alloc_table/iommu_tce_table_put there.
>>>>
>>>
>>> I don't see how to extract iommu_table_setparms_lpar() into
>>> iommu_table_setparms(), they look to be used for different machine
>>> types.
>>>
>>> Do mean you extracting most of iommu_table_setparms_lpar() (and maybe
>>> iommu_table_setparms() ) into a new helper, which is called in both
>>> functions and use it instead of iommu_table_update_window() ?
>>
>> Yes, this.
>
> I will do that then, seems better. :)
>
>>
>>
>>>>> +{
>>>>> + struct iommu_table *new_tbl, *old_tbl;
>>>>> +
>>>>> + new_tbl = iommu_pseries_alloc_table(nid);
>>>>> + if (!new_tbl)
>>>>> + return -ENOMEM;
>>>>> +
>>>>> + old_tbl = *tbl;
>>>>> + new_tbl->it_index = liobn;
>>>>> + new_tbl->it_offset = win_addr >> page_shift;
>>>>> + new_tbl->it_page_shift = page_shift;
>>>>> + new_tbl->it_size = window_size >> page_shift;
>>>>> + new_tbl->it_base = old_tbl->it_base;
>>>>
>>>> Should not be used in pseries.
>>>>
>>>
>>> The point here is to migrate the values from the older tbl to the
>>
>> The actual window/table is new (on the hypervisor side), you are not
>> migrating a single TCE, you deleted one whole window and created another
>> whole window, calling it "migration" is confusing, especially when PAPR
>> actually defines TCE migration.
>
> Ok, I understand it's confusing now. I will avoid using this term from
> now on.
>
>>
>>
>>> newer. I Would like to understand why this is bad, if it will still be
>>> 'unused' as the older tbl.
>>
>> Having explicit values is more readable imho.
>
> Ok, I understand why it should be improved.!
>
> Alexey, thank you for reviewing, and for helping me with my questions!
Thanks for doing this all!
>
> Best regards,
>
>>
>>
>>>>> + new_tbl->it_busno = old_tbl->it_busno;
>>>>> + new_tbl->it_blocksize = old_tbl->it_blocksize;
>>>>
>>>> 16 for pseries and does not change (may be even make it a macro).
>>>>
>>>>> + new_tbl->it_type = old_tbl->it_type;
>>>>
>>>> TCE_PCI.
>>>>
>>>
>>> Same as above.
>>>
>>>>> + new_tbl->it_ops = old_tbl->it_ops;
>>>>> +
>>>>> + iommu_init_table(new_tbl, nid, old_tbl->it_reserved_start, old_tbl->it_reserved_end);
>>>>> + iommu_tce_table_put(old_tbl);
>>>>> + *tbl = new_tbl;
>>>>>
>>>>> return 0;
>>>>> }
>>>>> @@ -1171,12 +1206,16 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
>>>>> struct direct_window *window;
>>>>> struct property *win64 = NULL;
>>>>> struct failed_ddw_pdn *fpdn;
>>>>> - bool default_win_removed = false;
>>>>> + bool default_win_removed = false, maps_whole_partition = false;
>>>>
>>>> s/maps_whole_partition/direct_mapping/
>>>>
>>>
>>> Sure, I will get it replaced.
>>>
>>>>> + struct pci_dn *pci = PCI_DN(pdn);
>>>>> + struct iommu_table *tbl = pci->table_group->tables[0];
>>>>>
>>>>> mutex_lock(&direct_window_init_mutex);
>>>>>
>>>>> - if (find_existing_ddw(pdn, &dev->dev.archdata.dma_offset))
>>>>> - goto out_unlock;
>>>>> + if (find_existing_ddw(pdn, &dev->dev.archdata.dma_offset, &maps_whole_partition)) {
>>>>> + mutex_unlock(&direct_window_init_mutex);
>>>>> + return maps_whole_partition;
>>>>> + }
>>>>>
>>>>> /*
>>>>> * If we already went through this for a previous function of
>>>>> @@ -1258,16 +1297,24 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
>>>>> query.page_size);
>>>>> goto out_failed;
>>>>> }
>>>>> +
>>>>> /* verify the window * number of ptes will map the partition */
>>>>> - /* check largest block * page size > max memory hotplug addr */
>>>>> max_addr = ddw_memory_hotplug_max();
>>>>> if (query.largest_available_block < (max_addr >> page_shift)) {
>>>>> - dev_dbg(&dev->dev, "can't map partition max 0x%llx with %llu "
>>>>> - "%llu-sized pages\n", max_addr, query.largest_available_block,
>>>>> - 1ULL << page_shift);
>>>>> - goto out_failed;
>>>>> + dev_dbg(&dev->dev, "can't map partition max 0x%llx with %llu %llu-sized pages\n",
>>>>> + max_addr, query.largest_available_block,
>>>>> + 1ULL << page_shift);
>>>>> +
>>>>> + len = order_base_2(query.largest_available_block << page_shift);
>>>>> + } else {
>>>>> + maps_whole_partition = true;
>>>>> + len = order_base_2(max_addr);
>>>>> }
>>>>> - len = order_base_2(max_addr);
>>>>> +
>>>>> + /* DDW + IOMMU on single window may fail if there is any allocation */
>>>>> + if (default_win_removed && !maps_whole_partition &&
>>>>> + iommu_table_in_use(tbl))
>>>>> + goto out_failed;
>>>>>
>>>>> ret = create_ddw(dev, ddw_avail, &create, page_shift, len);
>>>>> if (ret != 0)
>>>>> @@ -1277,8 +1324,8 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
>>>>> create.liobn, dn);
>>>>>
>>>>> win_addr = ((u64)create.addr_hi << 32) | create.addr_lo;
>>>>> - ret = ddw_property_create(&win64, DIRECT64_PROPNAME, create.liobn, win_addr,
>>>>> - page_shift, len);
>>>>> + ret = ddw_property_create(&win64, DMA64_PROPNAME, create.liobn, win_addr,
>>>>> + page_shift, len, maps_whole_partition);
>>>>> if (ret) {
>>>>> dev_info(&dev->dev,
>>>>> "couldn't allocate property, property name, or value\n");
>>>>> @@ -1297,12 +1344,25 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
>>>>> if (!window)
>>>>> goto out_prop_del;
>>>>>
>>>>> - ret = walk_system_ram_range(0, memblock_end_of_DRAM() >> PAGE_SHIFT,
>>>>> - win64->value, tce_setrange_multi_pSeriesLP_walk);
>>>>> - if (ret) {
>>>>> - dev_info(&dev->dev, "failed to map direct window for %pOF: %d\n",
>>>>> - dn, ret);
>>>>> - goto out_free_window;
>>>>> + if (maps_whole_partition) {
>>>>> + /* DDW maps the whole partition, so enable direct DMA mapping */
>>>>> + ret = walk_system_ram_range(0, memblock_end_of_DRAM() >> PAGE_SHIFT,
>>>>> + win64->value, tce_setrange_multi_pSeriesLP_walk);
>>>>> + if (ret) {
>>>>> + dev_info(&dev->dev, "failed to map direct window for %pOF: %d\n",
>>>>> + dn, ret);
>>>>> + goto out_free_window;
>>>>> + }
>>>>> + } else {
>>>>> + /* New table for using DDW instead of the default DMA window */
>>>>> + if (iommu_table_update_window(&tbl, pci->phb->node, create.liobn,
>>>>> + win_addr, page_shift, 1UL << len))
>>>>> + goto out_free_window;
>>>>> +
>>>>> + set_iommu_table_base(&dev->dev, tbl);
>>>>> + WARN_ON(dev->dev.archdata.dma_offset >= SZ_4G);
>>>>
>>>> What is this check for exactly? Why 4G, not >= 0, for example?
>>>
>>> I am not really sure, you suggested adding it here:
>>> http://patchwork.ozlabs.org/project/linuxppc-dev/patch/20200716071658.467820-6-leobras.c@gmail.com/#2488874
>>
>> Ah right I did suggest this :) My bad. I think I suggested it before
>> suggesting to keep the reserved area boundaries checked/adjusted to the
>> window boundaries, may as well drop this. Thanks,
>>
>>
>>> I can remove it if it's ok.
>>>
>>>>> + goto out_unlock;
>>>>> +
>>>>> }
>>>>>
>>>>> dev->dev.archdata.dma_offset = win_addr;
>>>>> @@ -1340,7 +1400,7 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
>>>>>
>>>>> out_unlock:
>>>>> mutex_unlock(&direct_window_init_mutex);
>>>>> - return win64;
>>>>> + return win64 && maps_whole_partition;
>>>>> }
>>>>>
>>>>> static void pci_dma_dev_setup_pSeriesLP(struct pci_dev *dev)
>>>>>
>
--
Alexey
^ permalink raw reply
* Re: [PATCH 12/14] x86: remove address space overrides using set_fs()
From: Al Viro @ 2020-09-04 2:55 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-arch, linuxppc-dev, Kees Cook, x86, linux-kernel,
Luis Chamberlain, linux-fsdevel, Linus Torvalds, Alexey Dobriyan
In-Reply-To: <20200903142242.925828-13-hch@lst.de>
On Thu, Sep 03, 2020 at 04:22:40PM +0200, Christoph Hellwig wrote:
> diff --git a/arch/x86/lib/getuser.S b/arch/x86/lib/getuser.S
> index c8a85b512796e1..94f7be4971ed04 100644
> --- a/arch/x86/lib/getuser.S
> +++ b/arch/x86/lib/getuser.S
> @@ -35,10 +35,19 @@
> #include <asm/smap.h>
> #include <asm/export.h>
>
> +#ifdef CONFIG_X86_5LEVEL
> +#define LOAD_TASK_SIZE_MINUS_N(n) \
> + ALTERNATIVE "mov $((1 << 47) - 4096 - (n)),%rdx", \
> + "mov $((1 << 56) - 4096 - (n)),%rdx", X86_FEATURE_LA57
> +#else
> +#define LOAD_TASK_SIZE_MINUS_N(n) \
> + mov $(TASK_SIZE_MAX - (n)),%_ASM_DX
> +#endif
Wait a sec... how is that supposed to build with X86_5LEVEL? Do you mean
#define LOAD_TASK_SIZE_MINUS_N(n) \
ALTERNATIVE __stringify(mov $((1 << 47) - 4096 - (n)),%rdx), \
__stringify(mov $((1 << 56) - 4096 - (n)),%rdx), X86_FEATURE_LA57
there?
^ permalink raw reply
* Re: [PATCH v4 08/13] mm/debug_vm_pgtable/locks: Move non page table modifying test together
From: Anshuman Khandual @ 2020-09-04 3:58 UTC (permalink / raw)
To: Aneesh Kumar K.V, linux-mm, akpm; +Cc: linuxppc-dev
In-Reply-To: <20200902114222.181353-9-aneesh.kumar@linux.ibm.com>
On 09/02/2020 05:12 PM, Aneesh Kumar K.V wrote:
> This will help in adding proper locks in a later patch
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> mm/debug_vm_pgtable.c | 51 ++++++++++++++++++++++++-------------------
> 1 file changed, 28 insertions(+), 23 deletions(-)
>
> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
> index de333871f407..f59cf6a9b05e 100644
> --- a/mm/debug_vm_pgtable.c
> +++ b/mm/debug_vm_pgtable.c
> @@ -986,7 +986,7 @@ static int __init debug_vm_pgtable(void)
> p4dp = p4d_alloc(mm, pgdp, vaddr);
> pudp = pud_alloc(mm, p4dp, vaddr);
> pmdp = pmd_alloc(mm, pudp, vaddr);
> - ptep = pte_alloc_map_lock(mm, pmdp, vaddr, &ptl);
> + ptep = pte_alloc_map(mm, pmdp, vaddr);
>
> /*
> * Save all the page table page addresses as the page table
> @@ -1006,33 +1006,12 @@ static int __init debug_vm_pgtable(void)
> p4d_basic_tests(p4d_aligned, prot);
> pgd_basic_tests(pgd_aligned, prot);
>
> - pte_clear_tests(mm, ptep, vaddr);
> - pmd_clear_tests(mm, pmdp);
> - pud_clear_tests(mm, pudp);
> - p4d_clear_tests(mm, p4dp);
> - pgd_clear_tests(mm, pgdp);
> -
> - pte_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
> - pmd_advanced_tests(mm, vma, pmdp, pmd_aligned, vaddr, prot);
> - pud_advanced_tests(mm, vma, pudp, pud_aligned, vaddr, prot);
> - hugetlb_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
> -
> pmd_leaf_tests(pmd_aligned, prot);
> pud_leaf_tests(pud_aligned, prot);
>
> - pmd_huge_tests(pmdp, pmd_aligned, prot);
> - pud_huge_tests(pudp, pud_aligned, prot);
> -
> pte_savedwrite_tests(pte_aligned, protnone);
> pmd_savedwrite_tests(pmd_aligned, protnone);
>
> - pte_unmap_unlock(ptep, ptl);
> -
> - pmd_populate_tests(mm, pmdp, saved_ptep);
> - pud_populate_tests(mm, pudp, saved_pmdp);
> - p4d_populate_tests(mm, p4dp, saved_pudp);
> - pgd_populate_tests(mm, pgdp, saved_p4dp);
> -
> pte_special_tests(pte_aligned, prot);
> pte_protnone_tests(pte_aligned, protnone);
> pmd_protnone_tests(pmd_aligned, protnone);
> @@ -1050,11 +1029,37 @@ static int __init debug_vm_pgtable(void)
> pmd_swap_tests(pmd_aligned, prot);
>
> swap_migration_tests();
> - hugetlb_basic_tests(pte_aligned, prot);
>
> pmd_thp_tests(pmd_aligned, prot);
> pud_thp_tests(pud_aligned, prot);
>
> + hugetlb_basic_tests(pte_aligned, prot);
> +
> + pte_clear_tests(mm, ptep, vaddr);
> + pmd_clear_tests(mm, pmdp);
> + pud_clear_tests(mm, pudp);
> + p4d_clear_tests(mm, p4dp);
> + pgd_clear_tests(mm, pgdp);
> +
> + ptl = pte_lockptr(mm, pmdp);
> + spin_lock(ptl);
> +
> + pte_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
> + pmd_advanced_tests(mm, vma, pmdp, pmd_aligned, vaddr, prot);
> + pud_advanced_tests(mm, vma, pudp, pud_aligned, vaddr, prot);
> + hugetlb_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
> +
> +
> + pmd_huge_tests(pmdp, pmd_aligned, prot);
> + pud_huge_tests(pudp, pud_aligned, prot);
> +
> + pte_unmap_unlock(ptep, ptl);
> +
> + pmd_populate_tests(mm, pmdp, saved_ptep);
> + pud_populate_tests(mm, pudp, saved_pmdp);
> + p4d_populate_tests(mm, p4dp, saved_pudp);
> + pgd_populate_tests(mm, pgdp, saved_p4dp);
> +
> p4d_free(mm, saved_p4dp);
> pud_free(mm, saved_pudp);
> pmd_free(mm, saved_pmdp);
>
Patches applied till here [i.e PATCH_1..PATCH_8] does not boot on arm64
platform, which might create a potential git bisect problem later on.
static void __init pte_advanced_tests(struct mm_struct *mm,
struct vm_area_struct *vma, pte_t *ptep,
unsigned long pfn, unsigned long vaddr,
pgprot_t prot)
{
pte_t pte = pfn_pte(pfn, prot);
/*
* Architectures optimize set_pte_at by avoiding TLB flush.
* This requires set_pte_at to be not used to update an
* existing pte entry. Clear pte before we do set_pte_at
*/
pr_debug("Validating PTE advanced\n");
pte = pfn_pte(pfn, prot);
set_pte_at(mm, vaddr, ptep, pte); ----------> Crashed here.
............
............
[ 19.031831] Unable to handle kernel paging request at virtual address fffffdfffde00028
[ 19.033181] Mem abort info:
[ 19.033627] ESR = 0x96000006
[ 19.034129] EC = 0x25: DABT (current EL), IL = 32 bits
[ 19.035002] SET = 0, FnV = 0
[ 19.035523] EA = 0, S1PTW = 0
[ 19.036054] Data abort info:
[ 19.036538] ISV = 0, ISS = 0x00000006
[ 19.037170] CM = 0, WnR = 0
[ 19.037662] swapper pgtable: 4k pages, 48-bit VAs, pgdp=000000008158b000
[ 19.038749] [fffffdfffde00028] pgd=0000000081d69003, p4d=0000000081d69003, pud=0000000081d6a003, pmd=0000000000000000
[ 19.040560] Internal error: Oops: 96000006 [#1] PREEMPT SMP
[ 19.041467] Modules linked in:
[ 19.041968] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G W 5.9.0-rc3-00231-gdef09f62540f #62
[ 19.043472] Hardware name: linux,dummy-virt (DT)
[ 19.044292] pstate: 20400005 (nzCv daif +PAN -UAO BTYPE=--)
[ 19.045235] pc : _raw_spin_lock+0x34/0x70
[ 19.045944] lr : debug_vm_pgtable+0x3c8/0x8b8
[ 19.046713] sp : ffff80001219bcf0
[ 19.047295] x29: ffff80001219bcf0 x28: ffff8000114ac000
[ 19.048224] x27: ffff0005df61f080 x26: ffff8000114ac000
[ 19.049148] x25: 0020000000000fd3 x24: 0020000081400fd3
[ 19.050076] x23: 00003df483b17000 x22: ffff0005ddd63e90
[ 19.051011] x21: ffff0005dddc8000 x20: ffff0005ddd5f0e8
[ 19.051937] x19: ffff0005de3718b8 x18: 0000000000000010
[ 19.052846] x17: 00000000dbb99b48 x16: 00000000cc138a43
[ 19.053774] x15: 0000000000000001 x14: 0000000000000002
[ 19.054703] x13: 000000000055e46d x12: fffffe0017577200
[ 19.055630] x11: 0000000000000008 x10: ffff0005fc455210
[ 19.056553] x9 : ffff0005fc455210 x8 : 0000000000000010
[ 19.057484] x7 : ffff8005ead60000 x6 : ffff0005fcfcee00
[ 19.058410] x5 : ffff0005fc455200 x4 : 0000000000000000
[ 19.059344] x3 : fffffdfffde00028 x2 : 0000000000000001
[ 19.060272] x1 : 0000000000000000 x0 : fffffdfffde00028
[ 19.061203] Call trace:
[ 19.061644] _raw_spin_lock+0x34/0x70
[ 19.062289] debug_vm_pgtable+0x3c8/0x8b8
[ 19.063001] do_one_initcall+0x74/0x1cc
[ 19.063680] kernel_init_freeable+0x1d0/0x238
[ 19.064448] kernel_init+0x14/0x118
[ 19.065067] ret_from_fork+0x10/0x34
[ 19.065703] Code: d503201f 52800001 52800022 2a0103e4 (88e47c02)
[ 19.066835] ---[ end trace ff33eeb13d2a13af ]---
^ permalink raw reply
* Re: [PATCH v4 03/13] mm/debug_vm_pgtable/ppc64: Avoid setting top bits in radom value
From: Anshuman Khandual @ 2020-09-04 4:03 UTC (permalink / raw)
To: Aneesh Kumar K.V, linux-mm, akpm; +Cc: linuxppc-dev
In-Reply-To: <20200902114222.181353-4-aneesh.kumar@linux.ibm.com>
On 09/02/2020 05:12 PM, Aneesh Kumar K.V wrote:
> ppc64 use bit 62 to indicate a pte entry (_PAGE_PTE). Avoid setting
> that bit in random value.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> mm/debug_vm_pgtable.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
> index 086309fb9b6f..00649b47f6e0 100644
> --- a/mm/debug_vm_pgtable.c
> +++ b/mm/debug_vm_pgtable.c
> @@ -44,10 +44,17 @@
> * entry type. But these bits might affect the ability to clear entries with
> * pxx_clear() because of how dynamic page table folding works on s390. So
> * while loading up the entries do not change the lower 4 bits. It does not
> - * have affect any other platform.
> + * have affect any other platform. Also avoid the 62nd bit on ppc64 that is
> + * used to mark a pte entry.
> */
> -#define S390_MASK_BITS 4
> -#define RANDOM_ORVALUE GENMASK(BITS_PER_LONG - 1, S390_MASK_BITS)
> +#define S390_SKIP_MASK GENMASK(3, 0)
> +#if __BITS_PER_LONG == 64
> +#define PPC64_SKIP_MASK GENMASK(62, 62)
> +#else
> +#define PPC64_SKIP_MASK 0x0
> +#endif
> +#define ARCH_SKIP_MASK (S390_SKIP_MASK | PPC64_SKIP_MASK)
> +#define RANDOM_ORVALUE (GENMASK(BITS_PER_LONG - 1, 0) & ~ARCH_SKIP_MASK)
> #define RANDOM_NZVALUE GENMASK(7, 0)
>
> static void __init pte_basic_tests(unsigned long pfn, pgprot_t prot)
>
Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
^ permalink raw reply
* Re: [PATCH v4 04/13] mm/debug_vm_pgtables/hugevmap: Use the arch helper to identify huge vmap support.
From: Anshuman Khandual @ 2020-09-04 4:08 UTC (permalink / raw)
To: Aneesh Kumar K.V, linux-mm, akpm; +Cc: linuxppc-dev
In-Reply-To: <20200902114222.181353-5-aneesh.kumar@linux.ibm.com>
On 09/02/2020 05:12 PM, Aneesh Kumar K.V wrote:
> ppc64 supports huge vmap only with radix translation. Hence use arch helper
> to determine the huge vmap support.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> mm/debug_vm_pgtable.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
> index 00649b47f6e0..4c73e63b4ceb 100644
> --- a/mm/debug_vm_pgtable.c
> +++ b/mm/debug_vm_pgtable.c
> @@ -28,6 +28,7 @@
> #include <linux/swapops.h>
> #include <linux/start_kernel.h>
> #include <linux/sched/mm.h>
> +#include <linux/io.h>
> #include <asm/pgalloc.h>
> #include <asm/tlbflush.h>
>
> @@ -206,11 +207,12 @@ static void __init pmd_leaf_tests(unsigned long pfn, pgprot_t prot)
> WARN_ON(!pmd_leaf(pmd));
> }
>
> +#ifdef CONFIG_HAVE_ARCH_HUGE_VMAP
> static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot)
> {
> pmd_t pmd;
>
> - if (!IS_ENABLED(CONFIG_HAVE_ARCH_HUGE_VMAP))
> + if (!arch_ioremap_pmd_supported())
> return;
>
> pr_debug("Validating PMD huge\n");
> @@ -224,6 +226,9 @@ static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot)
> pmd = READ_ONCE(*pmdp);
> WARN_ON(!pmd_none(pmd));
> }
> +#else /* CONFIG_HAVE_ARCH_HUGE_VMAP */
> +static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot) { }
> +#endif /* CONFIG_HAVE_ARCH_HUGE_VMAP */
>
> static void __init pmd_savedwrite_tests(unsigned long pfn, pgprot_t prot)
> {
> @@ -320,11 +325,12 @@ static void __init pud_leaf_tests(unsigned long pfn, pgprot_t prot)
> WARN_ON(!pud_leaf(pud));
> }
>
> +#ifdef CONFIG_HAVE_ARCH_HUGE_VMAP
> static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot)
> {
> pud_t pud;
>
> - if (!IS_ENABLED(CONFIG_HAVE_ARCH_HUGE_VMAP))
> + if (!arch_ioremap_pud_supported())
> return;
>
> pr_debug("Validating PUD huge\n");
> @@ -338,6 +344,10 @@ static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot)
> pud = READ_ONCE(*pudp);
> WARN_ON(!pud_none(pud));
> }
> +#else /* !CONFIG_HAVE_ARCH_HUGE_VMAP */
> +static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot) { }
> +#endif /* !CONFIG_HAVE_ARCH_HUGE_VMAP */
> +
> #else /* !CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD */
> static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot) { }
> static void __init pud_advanced_tests(struct mm_struct *mm,
>
Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
^ permalink raw reply
* Re: [PATCH v4 05/13] mm/debug_vm_pgtable/savedwrite: Enable savedwrite test with CONFIG_NUMA_BALANCING
From: Anshuman Khandual @ 2020-09-04 4:09 UTC (permalink / raw)
To: Aneesh Kumar K.V, linux-mm, akpm; +Cc: linuxppc-dev
In-Reply-To: <20200902114222.181353-6-aneesh.kumar@linux.ibm.com>
On 09/02/2020 05:12 PM, Aneesh Kumar K.V wrote:
> Saved write support was added to track the write bit of a pte after
> marking the pte protnone. This was done so that AUTONUMA can convert
> a write pte to protnone and still track the old write bit. When converting
> it back we set the pte write bit correctly thereby avoiding a write fault
> again. Hence enable the test only when CONFIG_NUMA_BALANCING is enabled and
> use protnone protflags.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> mm/debug_vm_pgtable.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
> index 4c73e63b4ceb..8704901f6bd8 100644
> --- a/mm/debug_vm_pgtable.c
> +++ b/mm/debug_vm_pgtable.c
> @@ -119,10 +119,14 @@ static void __init pte_savedwrite_tests(unsigned long pfn, pgprot_t prot)
> {
> pte_t pte = pfn_pte(pfn, prot);
>
> + if (!IS_ENABLED(CONFIG_NUMA_BALANCING))
> + return;
> +
> pr_debug("Validating PTE saved write\n");
> WARN_ON(!pte_savedwrite(pte_mk_savedwrite(pte_clear_savedwrite(pte))));
> WARN_ON(pte_savedwrite(pte_clear_savedwrite(pte_mk_savedwrite(pte))));
> }
> +
> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> static void __init pmd_basic_tests(unsigned long pfn, pgprot_t prot)
> {
> @@ -234,6 +238,9 @@ static void __init pmd_savedwrite_tests(unsigned long pfn, pgprot_t prot)
> {
> pmd_t pmd = pfn_pmd(pfn, prot);
>
> + if (!IS_ENABLED(CONFIG_NUMA_BALANCING))
> + return;
> +
> pr_debug("Validating PMD saved write\n");
> WARN_ON(!pmd_savedwrite(pmd_mk_savedwrite(pmd_clear_savedwrite(pmd))));
> WARN_ON(pmd_savedwrite(pmd_clear_savedwrite(pmd_mk_savedwrite(pmd))));
> @@ -1019,8 +1026,8 @@ static int __init debug_vm_pgtable(void)
> pmd_huge_tests(pmdp, pmd_aligned, prot);
> pud_huge_tests(pudp, pud_aligned, prot);
>
> - pte_savedwrite_tests(pte_aligned, prot);
> - pmd_savedwrite_tests(pmd_aligned, prot);
> + pte_savedwrite_tests(pte_aligned, protnone);
> + pmd_savedwrite_tests(pmd_aligned, protnone);
>
> pte_unmap_unlock(ptep, ptl);
>
>
No more checkpatch.pl warnings.
Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
^ permalink raw reply
* Re: [PATCH v4 13/13] mm/debug_vm_pgtable: Avoid none pte in pte_clear_test
From: Anshuman Khandual @ 2020-09-04 4:16 UTC (permalink / raw)
To: Aneesh Kumar K.V, linux-mm, akpm; +Cc: linuxppc-dev
In-Reply-To: <20200902114601.183715-1-aneesh.kumar@linux.ibm.com>
On 09/02/2020 05:16 PM, Aneesh Kumar K.V wrote:
> pte_clear_tests operate on an existing pte entry. Make sure that
> is not a none pte entry.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> mm/debug_vm_pgtable.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
> index 9afa1354326b..c36530c69e33 100644
> --- a/mm/debug_vm_pgtable.c
> +++ b/mm/debug_vm_pgtable.c
> @@ -542,9 +542,10 @@ static void __init pgd_populate_tests(struct mm_struct *mm, pgd_t *pgdp,
> #endif /* PAGETABLE_P4D_FOLDED */
>
> static void __init pte_clear_tests(struct mm_struct *mm, pte_t *ptep,
> - unsigned long vaddr)
> + unsigned long pfn, unsigned long vaddr,
> + pgprot_t prot)
> {
> - pte_t pte = ptep_get(ptep);
> + pte_t pte = pfn_pte(pfn, prot);
>
> pr_debug("Validating PTE clear\n");
> pte = __pte(pte_val(pte) | RANDOM_ORVALUE);
> @@ -1049,7 +1050,7 @@ static int __init debug_vm_pgtable(void)
>
> ptl = pte_lockptr(mm, pmdp);
> spin_lock(ptl);
> - pte_clear_tests(mm, ptep, vaddr);
> + pte_clear_tests(mm, ptep, pte_aligned, vaddr, prot);
> pte_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
> pte_unmap_unlock(ptep, ptl);
>
>
Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
^ permalink raw reply
* Re: [PATCH v4 10/13] mm/debug_vm_pgtable/thp: Use page table depost/withdraw with THP
From: Anshuman Khandual @ 2020-09-04 4:21 UTC (permalink / raw)
To: Aneesh Kumar K.V, linux-mm, akpm; +Cc: linuxppc-dev
In-Reply-To: <20200902114222.181353-11-aneesh.kumar@linux.ibm.com>
On 09/02/2020 05:12 PM, Aneesh Kumar K.V wrote:
> Architectures like ppc64 use deposited page table while updating the
> huge pte entries.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> mm/debug_vm_pgtable.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
> index 2bc1952e5f83..26023d990bd0 100644
> --- a/mm/debug_vm_pgtable.c
> +++ b/mm/debug_vm_pgtable.c
> @@ -154,7 +154,7 @@ static void __init pmd_basic_tests(unsigned long pfn, pgprot_t prot)
> static void __init pmd_advanced_tests(struct mm_struct *mm,
> struct vm_area_struct *vma, pmd_t *pmdp,
> unsigned long pfn, unsigned long vaddr,
> - pgprot_t prot)
> + pgprot_t prot, pgtable_t pgtable)
> {
> pmd_t pmd;
>
> @@ -165,6 +165,8 @@ static void __init pmd_advanced_tests(struct mm_struct *mm,
> /* Align the address wrt HPAGE_PMD_SIZE */
> vaddr = (vaddr & HPAGE_PMD_MASK) + HPAGE_PMD_SIZE;
>
> + pgtable_trans_huge_deposit(mm, pmdp, pgtable);
> +
> pmd = pmd_mkhuge(pfn_pmd(pfn, prot));
> set_pmd_at(mm, vaddr, pmdp, pmd);
> pmdp_set_wrprotect(mm, vaddr, pmdp);
> @@ -193,6 +195,8 @@ static void __init pmd_advanced_tests(struct mm_struct *mm,
> pmdp_test_and_clear_young(vma, vaddr, pmdp);
> pmd = READ_ONCE(*pmdp);
> WARN_ON(pmd_young(pmd));
> +
> + pgtable = pgtable_trans_huge_withdraw(mm, pmdp);
> }
>
> static void __init pmd_leaf_tests(unsigned long pfn, pgprot_t prot)
> @@ -371,7 +375,7 @@ static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot) { }
> static void __init pmd_advanced_tests(struct mm_struct *mm,
> struct vm_area_struct *vma, pmd_t *pmdp,
> unsigned long pfn, unsigned long vaddr,
> - pgprot_t prot)
> + pgprot_t prot, pgtable_t pgtable)
> {
> }
> static void __init pud_advanced_tests(struct mm_struct *mm,
> @@ -1048,7 +1052,7 @@ static int __init debug_vm_pgtable(void)
>
> ptl = pmd_lock(mm, pmdp);
> pmd_clear_tests(mm, pmdp);
> - pmd_advanced_tests(mm, vma, pmdp, pmd_aligned, vaddr, prot);
> + pmd_advanced_tests(mm, vma, pmdp, pmd_aligned, vaddr, prot, saved_ptep);
> pmd_huge_tests(pmdp, pmd_aligned, prot);
> pmd_populate_tests(mm, pmdp, saved_ptep);
> spin_unlock(ptl);
>
Moving this down further in the series has not really helped the possible
git bisect problem on arm64. Nonetheless, the patch in itself, makes sense.
Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
^ permalink raw reply
* Re: [PATCH 12/14] x86: remove address space overrides using set_fs()
From: Al Viro @ 2020-09-04 4:41 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-arch, linuxppc-dev, Kees Cook, x86, linux-kernel,
Luis Chamberlain, linux-fsdevel, Linus Torvalds, Alexey Dobriyan
In-Reply-To: <20200904025510.GO1236603@ZenIV.linux.org.uk>
On Fri, Sep 04, 2020 at 03:55:10AM +0100, Al Viro wrote:
> On Thu, Sep 03, 2020 at 04:22:40PM +0200, Christoph Hellwig wrote:
>
> > diff --git a/arch/x86/lib/getuser.S b/arch/x86/lib/getuser.S
> > index c8a85b512796e1..94f7be4971ed04 100644
> > --- a/arch/x86/lib/getuser.S
> > +++ b/arch/x86/lib/getuser.S
> > @@ -35,10 +35,19 @@
> > #include <asm/smap.h>
> > #include <asm/export.h>
> >
> > +#ifdef CONFIG_X86_5LEVEL
> > +#define LOAD_TASK_SIZE_MINUS_N(n) \
> > + ALTERNATIVE "mov $((1 << 47) - 4096 - (n)),%rdx", \
> > + "mov $((1 << 56) - 4096 - (n)),%rdx", X86_FEATURE_LA57
> > +#else
> > +#define LOAD_TASK_SIZE_MINUS_N(n) \
> > + mov $(TASK_SIZE_MAX - (n)),%_ASM_DX
> > +#endif
>
> Wait a sec... how is that supposed to build with X86_5LEVEL? Do you mean
>
> #define LOAD_TASK_SIZE_MINUS_N(n) \
> ALTERNATIVE __stringify(mov $((1 << 47) - 4096 - (n)),%rdx), \
> __stringify(mov $((1 << 56) - 4096 - (n)),%rdx), X86_FEATURE_LA57
>
> there?
Pushed out with the following folded in.
diff --git a/arch/x86/lib/getuser.S b/arch/x86/lib/getuser.S
index 94f7be4971ed..2f052bc96866 100644
--- a/arch/x86/lib/getuser.S
+++ b/arch/x86/lib/getuser.S
@@ -37,8 +37,8 @@
#ifdef CONFIG_X86_5LEVEL
#define LOAD_TASK_SIZE_MINUS_N(n) \
- ALTERNATIVE "mov $((1 << 47) - 4096 - (n)),%rdx", \
- "mov $((1 << 56) - 4096 - (n)),%rdx", X86_FEATURE_LA57
+ ALTERNATIVE __stringify(mov $((1 << 47) - 4096 - (n)),%rdx), \
+ __stringify(mov $((1 << 56) - 4096 - (n)),%rdx), X86_FEATURE_LA57
#else
#define LOAD_TASK_SIZE_MINUS_N(n) \
mov $(TASK_SIZE_MAX - (n)),%_ASM_DX
diff --git a/arch/x86/lib/putuser.S b/arch/x86/lib/putuser.S
index 445374885153..358239d77dff 100644
--- a/arch/x86/lib/putuser.S
+++ b/arch/x86/lib/putuser.S
@@ -33,8 +33,8 @@
#ifdef CONFIG_X86_5LEVEL
#define LOAD_TASK_SIZE_MINUS_N(n) \
- ALTERNATIVE "mov $((1 << 47) - 4096 - (n)),%rbx", \
- "mov $((1 << 56) - 4096 - (n)),%rbx", X86_FEATURE_LA57
+ ALTERNATIVE __stringify(mov $((1 << 47) - 4096 - (n)),%rbx), \
+ __stringify(mov $((1 << 56) - 4096 - (n)),%rbx), X86_FEATURE_LA57
#else
#define LOAD_TASK_SIZE_MINUS_N(n) \
mov $(TASK_SIZE_MAX - (n)),%_ASM_BX
^ permalink raw reply related
* Re: [PATCH v4 06/13] mm/debug_vm_pgtable/THP: Mark the pte entry huge before using set_pmd/pud_at
From: Anshuman Khandual @ 2020-09-04 5:21 UTC (permalink / raw)
To: Aneesh Kumar K.V, linux-mm, akpm; +Cc: linuxppc-dev
In-Reply-To: <20200902114222.181353-7-aneesh.kumar@linux.ibm.com>
On 09/02/2020 05:12 PM, Aneesh Kumar K.V wrote:
> kernel expects entries to be marked huge before we use
> set_pmd_at()/set_pud_at().
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> mm/debug_vm_pgtable.c | 20 +++++++++++---------
> 1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
> index 8704901f6bd8..9cafed39c236 100644
> --- a/mm/debug_vm_pgtable.c
> +++ b/mm/debug_vm_pgtable.c
> @@ -155,7 +155,7 @@ static void __init pmd_advanced_tests(struct mm_struct *mm,
> unsigned long pfn, unsigned long vaddr,
> pgprot_t prot)
> {
> - pmd_t pmd = pfn_pmd(pfn, prot);
> + pmd_t pmd;
>
> if (!has_transparent_hugepage())
> return;
> @@ -164,19 +164,19 @@ static void __init pmd_advanced_tests(struct mm_struct *mm,
> /* Align the address wrt HPAGE_PMD_SIZE */
> vaddr = (vaddr & HPAGE_PMD_MASK) + HPAGE_PMD_SIZE;
>
> - pmd = pfn_pmd(pfn, prot);
> + pmd = pmd_mkhuge(pfn_pmd(pfn, prot));
> set_pmd_at(mm, vaddr, pmdp, pmd);
> pmdp_set_wrprotect(mm, vaddr, pmdp);
> pmd = READ_ONCE(*pmdp);
> WARN_ON(pmd_write(pmd));
>
> - pmd = pfn_pmd(pfn, prot);
> + pmd = pmd_mkhuge(pfn_pmd(pfn, prot));
> set_pmd_at(mm, vaddr, pmdp, pmd);
> pmdp_huge_get_and_clear(mm, vaddr, pmdp);
> pmd = READ_ONCE(*pmdp);
> WARN_ON(!pmd_none(pmd));
>
> - pmd = pfn_pmd(pfn, prot);
> + pmd = pmd_mkhuge(pfn_pmd(pfn, prot));
> pmd = pmd_wrprotect(pmd);
> pmd = pmd_mkclean(pmd);
> set_pmd_at(mm, vaddr, pmdp, pmd);
> @@ -236,7 +236,7 @@ static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot)
>
> static void __init pmd_savedwrite_tests(unsigned long pfn, pgprot_t prot)
> {
> - pmd_t pmd = pfn_pmd(pfn, prot);
> + pmd_t pmd = pmd_mkhuge(pfn_pmd(pfn, prot));
>
> if (!IS_ENABLED(CONFIG_NUMA_BALANCING))
> return;
> @@ -276,7 +276,7 @@ static void __init pud_advanced_tests(struct mm_struct *mm,
> unsigned long pfn, unsigned long vaddr,
> pgprot_t prot)
> {
> - pud_t pud = pfn_pud(pfn, prot);
> + pud_t pud;
>
> if (!has_transparent_hugepage())
> return;
> @@ -285,25 +285,27 @@ static void __init pud_advanced_tests(struct mm_struct *mm,
> /* Align the address wrt HPAGE_PUD_SIZE */
> vaddr = (vaddr & HPAGE_PUD_MASK) + HPAGE_PUD_SIZE;
>
> + pud = pud_mkhuge(pfn_pud(pfn, prot));
> set_pud_at(mm, vaddr, pudp, pud);
> pudp_set_wrprotect(mm, vaddr, pudp);
> pud = READ_ONCE(*pudp);
> WARN_ON(pud_write(pud));
>
> #ifndef __PAGETABLE_PMD_FOLDED
> - pud = pfn_pud(pfn, prot);
> + pud = pud_mkhuge(pfn_pud(pfn, prot));
> set_pud_at(mm, vaddr, pudp, pud);
> pudp_huge_get_and_clear(mm, vaddr, pudp);
> pud = READ_ONCE(*pudp);
> WARN_ON(!pud_none(pud));
>
> - pud = pfn_pud(pfn, prot);
> + pud = pud_mkhuge(pfn_pud(pfn, prot));
> set_pud_at(mm, vaddr, pudp, pud);
> pudp_huge_get_and_clear_full(mm, vaddr, pudp, 1);
> pud = READ_ONCE(*pudp);
> WARN_ON(!pud_none(pud));
> #endif /* __PAGETABLE_PMD_FOLDED */
> - pud = pfn_pud(pfn, prot);
> +
> + pud = pud_mkhuge(pfn_pud(pfn, prot));
> pud = pud_wrprotect(pud);
> pud = pud_mkclean(pud);
> set_pud_at(mm, vaddr, pudp, pud);
>
Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
^ permalink raw reply
* Re: [PATCH v4 07/13] mm/debug_vm_pgtable/set_pte/pmd/pud: Don't use set_*_at to update an existing pte entry
From: Anshuman Khandual @ 2020-09-04 5:34 UTC (permalink / raw)
To: Aneesh Kumar K.V, linux-mm, akpm; +Cc: linuxppc-dev
In-Reply-To: <20200902114222.181353-8-aneesh.kumar@linux.ibm.com>
On 09/02/2020 05:12 PM, Aneesh Kumar K.V wrote:
> set_pte_at() should not be used to set a pte entry at locations that
> already holds a valid pte entry. Architectures like ppc64 don't do TLB
> invalidate in set_pte_at() and hence expect it to be used to set locations
> that are not a valid PTE.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> mm/debug_vm_pgtable.c | 35 +++++++++++++++--------------------
> 1 file changed, 15 insertions(+), 20 deletions(-)
>
> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
> index 9cafed39c236..de333871f407 100644
> --- a/mm/debug_vm_pgtable.c
> +++ b/mm/debug_vm_pgtable.c
> @@ -79,15 +79,18 @@ static void __init pte_advanced_tests(struct mm_struct *mm,
> {
> pte_t pte = pfn_pte(pfn, prot);
>
> + /*
> + * Architectures optimize set_pte_at by avoiding TLB flush.
> + * This requires set_pte_at to be not used to update an
> + * existing pte entry. Clear pte before we do set_pte_at
> + */
> +
> pr_debug("Validating PTE advanced\n");
> pte = pfn_pte(pfn, prot);
> set_pte_at(mm, vaddr, ptep, pte);
> ptep_set_wrprotect(mm, vaddr, ptep);
> pte = ptep_get(ptep);
> WARN_ON(pte_write(pte));
> -
> - pte = pfn_pte(pfn, prot);
> - set_pte_at(mm, vaddr, ptep, pte);
> ptep_get_and_clear(mm, vaddr, ptep);
> pte = ptep_get(ptep);
> WARN_ON(!pte_none(pte));
> @@ -101,13 +104,11 @@ static void __init pte_advanced_tests(struct mm_struct *mm,
> ptep_set_access_flags(vma, vaddr, ptep, pte, 1);
> pte = ptep_get(ptep);
> WARN_ON(!(pte_write(pte) && pte_dirty(pte)));
> -
> - pte = pfn_pte(pfn, prot);
> - set_pte_at(mm, vaddr, ptep, pte);
> ptep_get_and_clear_full(mm, vaddr, ptep, 1);
> pte = ptep_get(ptep);
> WARN_ON(!pte_none(pte));
>
> + pte = pfn_pte(pfn, prot);
> pte = pte_mkyoung(pte);
> set_pte_at(mm, vaddr, ptep, pte);
> ptep_test_and_clear_young(vma, vaddr, ptep);
> @@ -169,9 +170,6 @@ static void __init pmd_advanced_tests(struct mm_struct *mm,
> pmdp_set_wrprotect(mm, vaddr, pmdp);
> pmd = READ_ONCE(*pmdp);
> WARN_ON(pmd_write(pmd));
> -
> - pmd = pmd_mkhuge(pfn_pmd(pfn, prot));
> - set_pmd_at(mm, vaddr, pmdp, pmd);
> pmdp_huge_get_and_clear(mm, vaddr, pmdp);
> pmd = READ_ONCE(*pmdp);
> WARN_ON(!pmd_none(pmd));
> @@ -185,13 +183,11 @@ static void __init pmd_advanced_tests(struct mm_struct *mm,
> pmdp_set_access_flags(vma, vaddr, pmdp, pmd, 1);
> pmd = READ_ONCE(*pmdp);
> WARN_ON(!(pmd_write(pmd) && pmd_dirty(pmd)));
> -
> - pmd = pmd_mkhuge(pfn_pmd(pfn, prot));
> - set_pmd_at(mm, vaddr, pmdp, pmd);
> pmdp_huge_get_and_clear_full(vma, vaddr, pmdp, 1);
> pmd = READ_ONCE(*pmdp);
> WARN_ON(!pmd_none(pmd));
>
> + pmd = pmd_mkhuge(pfn_pmd(pfn, prot));
> pmd = pmd_mkyoung(pmd);
> set_pmd_at(mm, vaddr, pmdp, pmd);
> pmdp_test_and_clear_young(vma, vaddr, pmdp);
> @@ -292,17 +288,9 @@ static void __init pud_advanced_tests(struct mm_struct *mm,
> WARN_ON(pud_write(pud));
>
> #ifndef __PAGETABLE_PMD_FOLDED
> - pud = pud_mkhuge(pfn_pud(pfn, prot));
> - set_pud_at(mm, vaddr, pudp, pud);
> pudp_huge_get_and_clear(mm, vaddr, pudp);
> pud = READ_ONCE(*pudp);
> WARN_ON(!pud_none(pud));
> -
> - pud = pud_mkhuge(pfn_pud(pfn, prot));
> - set_pud_at(mm, vaddr, pudp, pud);
> - pudp_huge_get_and_clear_full(mm, vaddr, pudp, 1);
> - pud = READ_ONCE(*pudp);
> - WARN_ON(!pud_none(pud));
> #endif /* __PAGETABLE_PMD_FOLDED */
>
> pud = pud_mkhuge(pfn_pud(pfn, prot));
> @@ -315,6 +303,13 @@ static void __init pud_advanced_tests(struct mm_struct *mm,
> pud = READ_ONCE(*pudp);
> WARN_ON(!(pud_write(pud) && pud_dirty(pud)));
>
> +#ifndef __PAGETABLE_PMD_FOLDED
> + pudp_huge_get_and_clear_full(mm, vaddr, pudp, 1);
> + pud = READ_ONCE(*pudp);
> + WARN_ON(!pud_none(pud));
> +#endif /* __PAGETABLE_PMD_FOLDED */
> +
> + pud = pud_mkhuge(pfn_pud(pfn, prot));
> pud = pud_mkyoung(pud);
> set_pud_at(mm, vaddr, pudp, pud);
> pudp_test_and_clear_young(vma, vaddr, pudp);
>
Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
^ permalink raw reply
* Re: [PATCH v4 09/13] mm/debug_vm_pgtable/locks: Take correct page table lock
From: Anshuman Khandual @ 2020-09-04 5:39 UTC (permalink / raw)
To: Aneesh Kumar K.V, linux-mm, akpm; +Cc: linuxppc-dev
In-Reply-To: <20200902114222.181353-10-aneesh.kumar@linux.ibm.com>
On 09/02/2020 05:12 PM, Aneesh Kumar K.V wrote:
> Make sure we call pte accessors with correct lock held.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> mm/debug_vm_pgtable.c | 35 ++++++++++++++++++++++-------------
> 1 file changed, 22 insertions(+), 13 deletions(-)
>
> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
> index f59cf6a9b05e..2bc1952e5f83 100644
> --- a/mm/debug_vm_pgtable.c
> +++ b/mm/debug_vm_pgtable.c
> @@ -1035,30 +1035,39 @@ static int __init debug_vm_pgtable(void)
>
> hugetlb_basic_tests(pte_aligned, prot);
>
> - pte_clear_tests(mm, ptep, vaddr);
> - pmd_clear_tests(mm, pmdp);
> - pud_clear_tests(mm, pudp);
> - p4d_clear_tests(mm, p4dp);
> - pgd_clear_tests(mm, pgdp);
> + /*
> + * Page table modifying tests. They need to hold
> + * proper page table lock.
> + */
>
> ptl = pte_lockptr(mm, pmdp);
> spin_lock(ptl);
> -
> + pte_clear_tests(mm, ptep, vaddr);
> pte_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
> - pmd_advanced_tests(mm, vma, pmdp, pmd_aligned, vaddr, prot);
> - pud_advanced_tests(mm, vma, pudp, pud_aligned, vaddr, prot);
> - hugetlb_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
> -
> + pte_unmap_unlock(ptep, ptl);
>
> + ptl = pmd_lock(mm, pmdp);
> + pmd_clear_tests(mm, pmdp);
> + pmd_advanced_tests(mm, vma, pmdp, pmd_aligned, vaddr, prot);
> pmd_huge_tests(pmdp, pmd_aligned, prot);
> + pmd_populate_tests(mm, pmdp, saved_ptep);
> + spin_unlock(ptl);
> +
> + ptl = pud_lock(mm, pudp);
> + pud_clear_tests(mm, pudp);
> + pud_advanced_tests(mm, vma, pudp, pud_aligned, vaddr, prot);
> pud_huge_tests(pudp, pud_aligned, prot);
> + pud_populate_tests(mm, pudp, saved_pmdp);
> + spin_unlock(ptl);
>
> - pte_unmap_unlock(ptep, ptl);
> + hugetlb_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
>
> - pmd_populate_tests(mm, pmdp, saved_ptep);
> - pud_populate_tests(mm, pudp, saved_pmdp);
> + spin_lock(&mm->page_table_lock);
> + p4d_clear_tests(mm, p4dp);
> + pgd_clear_tests(mm, pgdp);
> p4d_populate_tests(mm, p4dp, saved_pudp);
> pgd_populate_tests(mm, pgdp, saved_p4dp);
> + spin_unlock(&mm->page_table_lock);
>
> p4d_free(mm, saved_p4dp);
> pud_free(mm, saved_pudp);
>
This patch, in itself looks good but will probably require folding with
the previous one to prevent the git bisect problem on arm64.
^ permalink raw reply
* Re: remove the last set_fs() in common code, and remove it for x86 and powerpc v3
From: Ingo Molnar @ 2020-09-04 6:00 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-arch, linuxppc-dev, Kees Cook, x86, linux-kernel,
Luis Chamberlain, Al Viro, linux-fsdevel, Linus Torvalds,
Alexey Dobriyan
In-Reply-To: <20200903142242.925828-1-hch@lst.de>
* Christoph Hellwig <hch@lst.de> wrote:
> Hi all,
>
> this series removes the last set_fs() used to force a kernel address
> space for the uaccess code in the kernel read/write/splice code, and then
> stops implementing the address space overrides entirely for x86 and
> powerpc.
Cool! For the x86 bits:
Acked-by: Ingo Molnar <mingo@kernel.org>
Thanks,
Ingo
^ permalink raw reply
* Re: [PATCH v4 11/13] mm/debug_vm_pgtable/pmd_clear: Don't use pmd/pud_clear on pte entries
From: Anshuman Khandual @ 2020-09-04 6:03 UTC (permalink / raw)
To: Aneesh Kumar K.V, linux-mm, akpm; +Cc: linuxppc-dev
In-Reply-To: <20200902114222.181353-12-aneesh.kumar@linux.ibm.com>
On 09/02/2020 05:12 PM, Aneesh Kumar K.V wrote:
> pmd_clear() should not be used to clear pmd level pte entries.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> mm/debug_vm_pgtable.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
> index 26023d990bd0..b53903fdee85 100644
> --- a/mm/debug_vm_pgtable.c
> +++ b/mm/debug_vm_pgtable.c
> @@ -196,6 +196,8 @@ static void __init pmd_advanced_tests(struct mm_struct *mm,
> pmd = READ_ONCE(*pmdp);
> WARN_ON(pmd_young(pmd));
>
> + /* Clear the pte entries */
> + pmdp_huge_get_and_clear(mm, vaddr, pmdp);
> pgtable = pgtable_trans_huge_withdraw(mm, pmdp);
> }
>
> @@ -319,6 +321,8 @@ static void __init pud_advanced_tests(struct mm_struct *mm,
> pudp_test_and_clear_young(vma, vaddr, pudp);
> pud = READ_ONCE(*pudp);
> WARN_ON(pud_young(pud));
> +
> + pudp_huge_get_and_clear(mm, vaddr, pudp);
> }
>
> static void __init pud_leaf_tests(unsigned long pfn, pgprot_t prot)
> @@ -442,8 +446,6 @@ static void __init pud_populate_tests(struct mm_struct *mm, pud_t *pudp,
> * This entry points to next level page table page.
> * Hence this must not qualify as pud_bad().
> */
> - pmd_clear(pmdp);
> - pud_clear(pudp);
> pud_populate(mm, pudp, pmdp);
> pud = READ_ONCE(*pudp);
> WARN_ON(pud_bad(pud));
> @@ -575,7 +577,6 @@ static void __init pmd_populate_tests(struct mm_struct *mm, pmd_t *pmdp,
> * This entry points to next level page table page.
> * Hence this must not qualify as pmd_bad().
> */
> - pmd_clear(pmdp);
> pmd_populate(mm, pmdp, pgtable);
> pmd = READ_ONCE(*pmdp);
> WARN_ON(pmd_bad(pmd));
>
Why pxxp_huge_get_and_clear() cannot be called inside pxx_populate_tests()
functions itself ? Nonetheless, this does not seem to cause any problem.
^ permalink raw reply
* Re: [PATCH v1 02/10] powerpc/kernel/iommu: Align size for IOMMU_PAGE_SIZE on iommu_*_coherent()
From: Leonardo Bras @ 2020-09-04 6:04 UTC (permalink / raw)
To: Alexey Kardashevskiy, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Christophe Leroy, Joel Stanley,
Thiago Jung Bauermann, Ram Pai, Brian King,
Murilo Fossa Vicentini, David Dai
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <8f569f68-5145-676e-50a1-b13f3fbd69cc@ozlabs.ru>
On Thu, 2020-09-03 at 14:41 +1000, Alexey Kardashevskiy wrote:
> I am new to this, so I am trying to understand how a memory page mapped
> > as DMA, and used for something else could be a problem.
>
> From the device prospective, there is PCI space and everything from 0
> till 1<<64 is accessible and what is that mapped to - the device does
> not know. PHB's IOMMU is the thing to notice invalid access and raise
> EEH but PHB only knows about PCI->physical memory mapping (with IOMMU
> pages) but nothing about the host kernel pages. Does this help? Thanks,
According to our conversation on Slack:
1- There is a problem if a hypervisor gives to it's VMs contiguous
memory blocks that are not aligned to IOMMU pages, because then an
iommu_map_page() could map some memory in this VM and some memory in
other VM / process.
2- To guarantee this, we should have system pagesize >= iommu_pagesize
One way to get (2) is by doing this in enable_ddw():
if ((query.page_size & 4) && PAGE_SHIFT >= 24) {
page_shift = 24; /* 16MB */
} else if ((query.page_size & 2) && PAGE_SHIFT >= 16 ) {
page_shift = 16; /* 64kB */
} else if (query.page_size & 1 && PAGE_SHIFT >= 12) {
page_shift = 12; /* 4kB */
[...]
Another way of solving this, would be adding in LoPAR documentation
that the blocksize of contiguous memory the hypervisor gives a VM
should always be aligned to IOMMU pagesize offered.
I think the best approach would be first sending the above patch, which
is faster, and then get working into adding that to documentation, so
hypervisors guarantee this.
If this gets into the docs, we can revert the patch.
What do you think?
Best regards!
^ permalink raw reply
* Re: [PATCH v4 12/13] mm/debug_vm_pgtable/hugetlb: Disable hugetlb test on ppc64
From: Anshuman Khandual @ 2020-09-04 6:19 UTC (permalink / raw)
To: Aneesh Kumar K.V, linux-mm, akpm; +Cc: linuxppc-dev
In-Reply-To: <20200902114222.181353-13-aneesh.kumar@linux.ibm.com>
On 09/02/2020 05:12 PM, Aneesh Kumar K.V wrote:
> The seems to be missing quite a lot of details w.r.t allocating
> the correct pgtable_t page (huge_pte_alloc()), holding the right
> lock (huge_pte_lock()) etc. The vma used is also not a hugetlb VMA.
>
> ppc64 do have runtime checks within CONFIG_DEBUG_VM for most of these.
> Hence disable the test on ppc64.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> mm/debug_vm_pgtable.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
> index b53903fdee85..9afa1354326b 100644
> --- a/mm/debug_vm_pgtable.c
> +++ b/mm/debug_vm_pgtable.c
> @@ -811,6 +811,7 @@ static void __init hugetlb_basic_tests(unsigned long pfn, pgprot_t prot)
> #endif /* CONFIG_ARCH_WANT_GENERAL_HUGETLB */
> }
>
> +#ifndef CONFIG_PPC_BOOK3S_64
> static void __init hugetlb_advanced_tests(struct mm_struct *mm,
> struct vm_area_struct *vma,
> pte_t *ptep, unsigned long pfn,
> @@ -853,6 +854,7 @@ static void __init hugetlb_advanced_tests(struct mm_struct *mm,
> pte = huge_ptep_get(ptep);
> WARN_ON(!(huge_pte_write(pte) && huge_pte_dirty(pte)));
> }
> +#endif
> #else /* !CONFIG_HUGETLB_PAGE */
> static void __init hugetlb_basic_tests(unsigned long pfn, pgprot_t prot) { }
> static void __init hugetlb_advanced_tests(struct mm_struct *mm,
> @@ -1065,7 +1067,9 @@ static int __init debug_vm_pgtable(void)
> pud_populate_tests(mm, pudp, saved_pmdp);
> spin_unlock(ptl);
>
> +#ifndef CONFIG_PPC_BOOK3S_64
> hugetlb_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
> +#endif
>
> spin_lock(&mm->page_table_lock);
> p4d_clear_tests(mm, p4dp);
>
Is it still required now that DEBUG_VM_PGTABLE has been dropped from powerpc
or you would like to re-enabled it back ?
https://lore.kernel.org/linuxppc-dev/159913592797.5893.5829441560236719450.b4-ty@ellerman.id.au/T/#m6d890e2fe84cf180cb875fae5f791e9c83db8d30
^ permalink raw reply
* Re: [PATCH 12/14] x86: remove address space overrides using set_fs()
From: Christoph Hellwig @ 2020-09-04 6:38 UTC (permalink / raw)
To: Al Viro
Cc: linux-arch, linuxppc-dev, Kees Cook, x86, linux-kernel,
Alexey Dobriyan, Luis Chamberlain, linux-fsdevel, Linus Torvalds,
Christoph Hellwig
In-Reply-To: <20200904025510.GO1236603@ZenIV.linux.org.uk>
On Fri, Sep 04, 2020 at 03:55:10AM +0100, Al Viro wrote:
> On Thu, Sep 03, 2020 at 04:22:40PM +0200, Christoph Hellwig wrote:
>
> > diff --git a/arch/x86/lib/getuser.S b/arch/x86/lib/getuser.S
> > index c8a85b512796e1..94f7be4971ed04 100644
> > --- a/arch/x86/lib/getuser.S
> > +++ b/arch/x86/lib/getuser.S
> > @@ -35,10 +35,19 @@
> > #include <asm/smap.h>
> > #include <asm/export.h>
> >
> > +#ifdef CONFIG_X86_5LEVEL
> > +#define LOAD_TASK_SIZE_MINUS_N(n) \
> > + ALTERNATIVE "mov $((1 << 47) - 4096 - (n)),%rdx", \
> > + "mov $((1 << 56) - 4096 - (n)),%rdx", X86_FEATURE_LA57
> > +#else
> > +#define LOAD_TASK_SIZE_MINUS_N(n) \
> > + mov $(TASK_SIZE_MAX - (n)),%_ASM_DX
> > +#endif
>
> Wait a sec... how is that supposed to build with X86_5LEVEL? Do you mean
>
> #define LOAD_TASK_SIZE_MINUS_N(n) \
> ALTERNATIVE __stringify(mov $((1 << 47) - 4096 - (n)),%rdx), \
> __stringify(mov $((1 << 56) - 4096 - (n)),%rdx), X86_FEATURE_LA57
>
> there?
Don't ask me about the how, but it builds and works with X86_5LEVEL,
and the style is copied from elsewhere..
^ permalink raw reply
* Re: [PATCH v4 00/13] mm/debug_vm_pgtable fixes
From: Anshuman Khandual @ 2020-09-04 6:48 UTC (permalink / raw)
To: Aneesh Kumar K.V, linux-mm, akpm
Cc: linux-s390@vger.kernel.org, Vineet Gupta, linux-riscv,
linux-snps-arc@lists.infradead.org, linuxppc-dev, Gerald Schaefer
In-Reply-To: <20200902114222.181353-1-aneesh.kumar@linux.ibm.com>
On 09/02/2020 05:12 PM, Aneesh Kumar K.V wrote:
> This patch series includes fixes for debug_vm_pgtable test code so that
> they follow page table updates rules correctly. The first two patches introduce
> changes w.r.t ppc64. The patches are included in this series for completeness. We can
> merge them via ppc64 tree if required.
>
> Hugetlb test is disabled on ppc64 because that needs larger change to satisfy
> page table update rules.
>
> These tests are broken w.r.t page table update rules and results in kernel
> crash as below.
>
> [ 21.083519] kernel BUG at arch/powerpc/mm/pgtable.c:304!
> cpu 0x0: Vector: 700 (Program Check) at [c000000c6d1e76c0]
> pc: c00000000009a5ec: assert_pte_locked+0x14c/0x380
> lr: c0000000005eeeec: pte_update+0x11c/0x190
> sp: c000000c6d1e7950
> msr: 8000000002029033
> current = 0xc000000c6d172c80
> paca = 0xc000000003ba0000 irqmask: 0x03 irq_happened: 0x01
> pid = 1, comm = swapper/0
> kernel BUG at arch/powerpc/mm/pgtable.c:304!
> [link register ] c0000000005eeeec pte_update+0x11c/0x190
> [c000000c6d1e7950] 0000000000000001 (unreliable)
> [c000000c6d1e79b0] c0000000005eee14 pte_update+0x44/0x190
> [c000000c6d1e7a10] c000000001a2ca9c pte_advanced_tests+0x160/0x3d8
> [c000000c6d1e7ab0] c000000001a2d4fc debug_vm_pgtable+0x7e8/0x1338
> [c000000c6d1e7ba0] c0000000000116ec do_one_initcall+0xac/0x5f0
> [c000000c6d1e7c80] c0000000019e4fac kernel_init_freeable+0x4dc/0x5a4
> [c000000c6d1e7db0] c000000000012474 kernel_init+0x24/0x160
> [c000000c6d1e7e20] c00000000000cbd0 ret_from_kernel_thread+0x5c/0x6c
>
> With DEBUG_VM disabled
>
> [ 20.530152] BUG: Kernel NULL pointer dereference on read at 0x00000000
> [ 20.530183] Faulting instruction address: 0xc0000000000df330
> cpu 0x33: Vector: 380 (Data SLB Access) at [c000000c6d19f700]
> pc: c0000000000df330: memset+0x68/0x104
> lr: c00000000009f6d8: hash__pmdp_huge_get_and_clear+0xe8/0x1b0
> sp: c000000c6d19f990
> msr: 8000000002009033
> dar: 0
> current = 0xc000000c6d177480
> paca = 0xc00000001ec4f400 irqmask: 0x03 irq_happened: 0x01
> pid = 1, comm = swapper/0
> [link register ] c00000000009f6d8 hash__pmdp_huge_get_and_clear+0xe8/0x1b0
> [c000000c6d19f990] c00000000009f748 hash__pmdp_huge_get_and_clear+0x158/0x1b0 (unreliable)
> [c000000c6d19fa10] c0000000019ebf30 pmd_advanced_tests+0x1f0/0x378
> [c000000c6d19fab0] c0000000019ed088 debug_vm_pgtable+0x79c/0x1244
> [c000000c6d19fba0] c0000000000116ec do_one_initcall+0xac/0x5f0
> [c000000c6d19fc80] c0000000019a4fac kernel_init_freeable+0x4dc/0x5a4
> [c000000c6d19fdb0] c000000000012474 kernel_init+0x24/0x160
> [c000000c6d19fe20] c00000000000cbd0 ret_from_kernel_thread+0x5c/0x6c
>
> Changes from v3:
> * Address review feedback
> * Move page table depost and withdraw patch after adding pmdlock to avoid bisect failure.
This version
- Builds on x86, arm64, s390, arc, powerpc and riscv (defconfig with DEBUG_VM_PGTABLE)
- Runs on arm64 and x86 without any regression, atleast nothing that I have noticed
- Will be great if this could get tested on s390, arc, riscv, ppc32 platforms as well
+ linux-riscv <linux-riscv@lists.infradead.org>
+ linux-snps-arc@lists.infradead.org <linux-snps-arc@lists.infradead.org>
+ linux-s390@vger.kernel.org
+ Gerald Schaefer <gerald.schaefer@de.ibm.com>
+ Vineet Gupta <vgupta@synopsys.com>
There is still an open git bisect issue on arm64 platform which ideally should be fixed.
- Anshuman
^ permalink raw reply
* Re: [PATCH 12/14] x86: remove address space overrides using set_fs()
From: Christoph Hellwig @ 2020-09-04 7:47 UTC (permalink / raw)
To: Al Viro
Cc: linux-arch, linuxppc-dev, Kees Cook, x86, linux-kernel,
Alexey Dobriyan, Luis Chamberlain, linux-fsdevel, Linus Torvalds,
Christoph Hellwig
In-Reply-To: <20200904063813.GA12204@lst.de>
On Fri, Sep 04, 2020 at 08:38:13AM +0200, Christoph Hellwig wrote:
> > Wait a sec... how is that supposed to build with X86_5LEVEL? Do you mean
> >
> > #define LOAD_TASK_SIZE_MINUS_N(n) \
> > ALTERNATIVE __stringify(mov $((1 << 47) - 4096 - (n)),%rdx), \
> > __stringify(mov $((1 << 56) - 4096 - (n)),%rdx), X86_FEATURE_LA57
> >
> > there?
>
> Don't ask me about the how, but it builds and works with X86_5LEVEL,
> and the style is copied from elsewhere..
Actually, it doesn't any more. Looks like the change to pass the n
parameter as suggested by Linus broke the previously working version.
^ permalink raw reply
* RE: [PATCH 12/14] x86: remove address space overrides using set_fs()
From: David Laight @ 2020-09-04 7:59 UTC (permalink / raw)
To: 'Linus Torvalds'
Cc: linux-arch@vger.kernel.org, Kees Cook, x86@kernel.org,
linux-kernel@vger.kernel.org, Christoph Hellwig, Luis Chamberlain,
Al Viro, linux-fsdevel@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org, Alexey Dobriyan
In-Reply-To: <CAHk-=whDtnudkbZ8-hR8HiDE7zog0dv+Gu9Sx5i6SPakrDtajQ@mail.gmail.com>
From: Linus Torvalds
> Sent: 04 September 2020 00:26
>
> On Thu, Sep 3, 2020 at 2:30 PM David Laight <David.Laight@aculab.com> wrote:
> >
> > A non-canonical (is that the right term) address between the highest
> > valid user address and the lowest valid kernel address (7ffe to fffe?)
> > will fault anyway.
>
> Yes.
>
> But we actually warn against that fault, because it's been a good way
> to catch places that didn't use the proper "access_ok()" pattern.
>
> See ex_handler_uaccess() and the
>
> WARN_ONCE(trapnr == X86_TRAP_GP, "General protection fault in
> user access. Non-canonical address?");
>
> warning. It's been good for randomized testing - a missing range check
> on a user address will often hit this.
>
> Of course, you should never see it in real life (and hopefully not in
> testing either any more). But belt-and-suspenders..
That could still be effective, just pick an address limit that is
appropriate for the one access_ok() is using.
Even if access_ok() uses 1<<63 there are plenty of addresses above it that fault.
But the upper limit for 5-level page tables could be used all the time.
One option is to test '(address | length) < (3<<62)' in access_ok().
That is also moderately suitable for masking invalid addresses to 0.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: ptrace_syscall_32 is failing
From: Thomas Gleixner @ 2020-09-04 10:13 UTC (permalink / raw)
To: Andy Lutomirski
Cc: linux-s390, linuxppc-dev, Vasily Gorbik, Brian Gerst,
Heiko Carstens, X86 ML, LKML, Christian Borntraeger,
Paul Mackerras, Catalin Marinas, Andy Lutomirski, Will Deacon,
linux-arm-kernel
In-Reply-To: <CALCETrUuyXpG0Vhrb-9m-G8J94+2bGqdrJkKfz+-5z7dsGLK8Q@mail.gmail.com>
Andy,
On Wed, Sep 02 2020 at 09:49, Andy Lutomirski wrote:
> On Wed, Sep 2, 2020 at 1:29 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>>
>> But you might tell me where exactly you want to inject the SIGTRAP in
>> the syscall exit code flow.
>
> It would be a bit complicated. Definitely after any signals from the
> syscall are delivered. Right now, I think that we don't deliver a
> SIGTRAP on the instruction boundary after SYSCALL while
> single-stepping. (I think we used to, but only sometimes, and now we
> are at least consistent.) This is because IRET will not trap if it
> starts with TF clear and ends up setting it. (I asked Intel to
> document this, and I think they finally did, although I haven't gotten
> around to reading the new docs. Certainly the old docs as of a year
> or two ago had no description whatsoever of how TF changes worked.)
>
> Deciding exactly *when* a trap should occur would be nontrivial -- we
> can't trap on sigreturn() from a SIGTRAP, for example.
>
> So this isn't fully worked out.
Oh well.
>> >> I don't think we want that in general. The current variant is perfectly
>> >> fine for everything except the 32bit fast syscall nonsense. Also
>> >> irqentry_entry/exit is not equivalent to the syscall_enter/exit
>> >> counterparts.
>> >
>> > If there are any architectures in which actual work is needed to
>> > figure out whether something is a syscall in the first place, they'll
>> > want to do the usual kernel entry work before the syscall entry work.
>>
>> That's low level entry code which does not require RCU, lockdep, tracing
>> or whatever muck we setup before actual work can be done.
>>
>> arch_asm_entry()
>> ...
>> arch_c_entry(cause) {
>> switch(cause) {
>> case EXCEPTION: arch_c_exception(...);
>> case SYSCALL: arch_c_syscall(...);
>> ...
>> }
>
> You're assuming that figuring out the cause doesn't need the kernel
> entry code to run first. In the case of the 32-bit vDSO fast
> syscalls, we arguably don't know whether an entry is a syscall until
> we have done a user memory access. Logically, we're doing:
>
> if (get_user() < 0) {
> /* Not a syscall. This is actually a silly operation that sets AX =
> -EFAULT and returns. Do not audit or invoke ptrace. */
> } else {
> /* This actually is a syscall. */
> }
Yes, that's what I've addressed with providing split interfaces.
>> You really want to differentiate between exception and syscall
>> entry/exit.
>>
>
> Why do we want to distinguish between exception and syscall
> entry/exit? For the enter part, AFAICS the exception case boils down
> to enter_from_user_mode() and the syscall case is:
>
> enter_from_user_mode(regs);
> instrumentation_begin();
>
> local_irq_enable();
> ti_work = READ_ONCE(current_thread_info()->flags);
> if (ti_work & SYSCALL_ENTER_WORK)
> syscall = syscall_trace_enter(regs, syscall, ti_work);
> instrumentation_end();
>
> Which would decompose quite nicely as a regular (non-syscall) entry
> plus the syscall part later.
There is a difference between syscall entry and exception entry at least
in my view:
syscall:
enter_from_user_mode(regs);
local_irq_enable();
exception:
enter_from_user_mode(regs);
>> we'd have:
>>
>> arch_c_entry()
>> irqentry_enter();
>> local_irq_enble();
>> nr = syscall_enter_from_user_mode_work();
>> ...
>>
>> which enforces two calls for sane entries and more code in arch/....
>
> This is why I still like my:
>
> arch_c_entry()
> irqentry_enter_from_user_mode();
> generic_syscall();
> exit...
So what we have now (with my patch applied) is either:
1) arch_c_entry()
nr = syscall_enter_from_user_mode();
arch_handle_syscall(nr);
syscall_exit_to_user_mode();
or for that extra 32bit fast syscall thing:
2) arch_c_entry()
syscall_enter_from_user_mode_prepare();
arch_do_stuff();
nr = syscall_enter_from_user_mode_work();
arch_handle_syscall(nr);
syscall_exit_to_user_mode();
So for sane cases you just use #1.
Ideally we'd not need arch_handle_syscall(nr) at all, but that does not
work with multiple ABIs supported, i.e. the compat muck.
The only way we could make that work is to have:
syscall_enter_exit(regs, mode)
nr = syscall_enter_from_user_mode();
arch_handle_syscall(mode, nr);
syscall_exit_to_user_mode();
and then arch_c_entry() becomes:
syscall_enter_exit(regs, mode);
which means that arch_handle_syscall() would have to evaluate the mode
and chose the appropriate syscall table. Not sure whether that's a win.
Thanks,
tglx
^ permalink raw reply
* [PATCH] powerpc/uaccess: Add pre-update addressing to __put_user_asm_goto()
From: Christophe Leroy @ 2020-09-04 10:46 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
Enable pre-update addressing mode in __put_user_asm_goto()
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/uaccess.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 7c2427f237e1..a5cfe867fbdc 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -254,7 +254,7 @@ do { \
"1: " op "%U1%X1 %0,%1 # put_user\n" \
EX_TABLE(1b, %l2) \
: \
- : "r" (x), "m" (*addr) \
+ : "r" (x), "m<>" (*addr) \
: \
: label)
--
2.25.0
^ permalink raw reply related
* [PATCH 1/3] powerpc/uaccess: Switch __put_user_size_allowed() to __put_user_asm_goto()
From: Christophe Leroy @ 2020-09-04 11:01 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
__put_user_asm_goto() provides more flexibility to GCC and avoids using
a local variable to tell if the write succeeded or not.
GCC can then avoid implementing a cmp in the fast path.
See the difference for a small function like the PPC64 version of
save_general_regs() in arch/powerpc/kernel/signal_32.c:
Before the patch (unreachable nop removed):
0000000000000c10 <.save_general_regs>:
c10: 39 20 00 2c li r9,44
c14: 39 40 00 00 li r10,0
c18: 7d 29 03 a6 mtctr r9
c1c: 38 c0 00 00 li r6,0
c20: 48 00 00 14 b c34 <.save_general_regs+0x24>
c30: 42 40 00 40 bdz c70 <.save_general_regs+0x60>
c34: 28 2a 00 27 cmpldi r10,39
c38: 7c c8 33 78 mr r8,r6
c3c: 79 47 1f 24 rldicr r7,r10,3,60
c40: 39 20 00 01 li r9,1
c44: 41 82 00 0c beq c50 <.save_general_regs+0x40>
c48: 7d 23 38 2a ldx r9,r3,r7
c4c: 79 29 00 20 clrldi r9,r9,32
c50: 91 24 00 00 stw r9,0(r4)
c54: 2c 28 00 00 cmpdi r8,0
c58: 39 4a 00 01 addi r10,r10,1
c5c: 38 84 00 04 addi r4,r4,4
c60: 41 82 ff d0 beq c30 <.save_general_regs+0x20>
c64: 38 60 ff f2 li r3,-14
c68: 4e 80 00 20 blr
c70: 38 60 00 00 li r3,0
c74: 4e 80 00 20 blr
0000000000000000 <.fixup>:
cc: 39 00 ff f2 li r8,-14
d0: 48 00 00 00 b d0 <.fixup+0xd0>
d0: R_PPC64_REL24 .text+0xc54
After the patch:
0000000000001490 <.save_general_regs>:
1490: 39 20 00 2c li r9,44
1494: 39 40 00 00 li r10,0
1498: 7d 29 03 a6 mtctr r9
149c: 60 00 00 00 nop
14a0: 28 2a 00 27 cmpldi r10,39
14a4: 79 48 1f 24 rldicr r8,r10,3,60
14a8: 39 20 00 01 li r9,1
14ac: 41 82 00 0c beq 14b8 <.save_general_regs+0x28>
14b0: 7d 23 40 2a ldx r9,r3,r8
14b4: 79 29 00 20 clrldi r9,r9,32
14b8: 91 24 00 00 stw r9,0(r4)
14bc: 39 4a 00 01 addi r10,r10,1
14c0: 38 84 00 04 addi r4,r4,4
14c4: 42 00 ff dc bdnz 14a0 <.save_general_regs+0x10>
14c8: 38 60 00 00 li r3,0
14cc: 4e 80 00 20 blr
14d0: 38 60 ff f2 li r3,-14
14d4: 4e 80 00 20 blr
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/uaccess.h | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index a5cfe867fbdc..96d1c144f92b 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -189,14 +189,14 @@ extern long __put_user_bad(void);
#define __put_user_size_allowed(x, ptr, size, retval) \
do { \
+ __label__ __pu_failed; \
+ \
retval = 0; \
- switch (size) { \
- case 1: __put_user_asm(x, ptr, retval, "stb"); break; \
- case 2: __put_user_asm(x, ptr, retval, "sth"); break; \
- case 4: __put_user_asm(x, ptr, retval, "stw"); break; \
- case 8: __put_user_asm2(x, ptr, retval); break; \
- default: __put_user_bad(); \
- } \
+ __put_user_size_goto(x, ptr, size, __pu_failed); \
+ break; \
+ \
+__pu_failed: \
+ retval = -EFAULT; \
} while (0)
#define __put_user_size(x, ptr, size, retval) \
--
2.25.0
^ permalink raw reply related
* [PATCH 2/3] powerpc/uaccess: Switch __patch_instruction() to __put_user_asm_goto()
From: Christophe Leroy @ 2020-09-04 11:01 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <94ba5a5138f99522e1562dbcdb38d31aa790dc89.1599216721.git.christophe.leroy@csgroup.eu>
__patch_instruction() is the only user of __put_user_asm() outside
of asm/uaccess.h
Switch to the new __put_user_asm_goto() to enable retirement of
__put_user_asm() in a later patch.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/lib/code-patching.c | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 8c3934ea6220..2333625b5e31 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -21,21 +21,18 @@
static int __patch_instruction(struct ppc_inst *exec_addr, struct ppc_inst instr,
struct ppc_inst *patch_addr)
{
- int err = 0;
-
- if (!ppc_inst_prefixed(instr)) {
- __put_user_asm(ppc_inst_val(instr), patch_addr, err, "stw");
- } else {
- __put_user_asm(ppc_inst_as_u64(instr), patch_addr, err, "std");
- }
-
- if (err)
- return err;
+ if (!ppc_inst_prefixed(instr))
+ __put_user_asm_goto(ppc_inst_val(instr), patch_addr, failed, "stw");
+ else
+ __put_user_asm_goto(ppc_inst_as_u64(instr), patch_addr, failed, "std");
asm ("dcbst 0, %0; sync; icbi 0,%1; sync; isync" :: "r" (patch_addr),
"r" (exec_addr));
return 0;
+
+failed:
+ return -EFAULT;
}
int raw_patch_instruction(struct ppc_inst *addr, struct ppc_inst instr)
--
2.25.0
^ permalink raw reply related
* [PATCH 3/3] powerpc/uaccess: Remove __put_user_asm() and __put_user_asm2()
From: Christophe Leroy @ 2020-09-04 11:01 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <94ba5a5138f99522e1562dbcdb38d31aa790dc89.1599216721.git.christophe.leroy@csgroup.eu>
__put_user_asm() and __put_user_asm2() are not used anymore.
Remove them.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/uaccess.h | 41 ++++--------------------------
1 file changed, 5 insertions(+), 36 deletions(-)
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 96d1c144f92b..26781b044932 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -151,42 +151,6 @@ static inline int __access_ok(unsigned long addr, unsigned long size,
extern long __put_user_bad(void);
-/*
- * We don't tell gcc that we are accessing memory, but this is OK
- * because we do not write to any memory gcc knows about, so there
- * are no aliasing issues.
- */
-#define __put_user_asm(x, addr, err, op) \
- __asm__ __volatile__( \
- "1: " op "%U2%X2 %1,%2 # put_user\n" \
- "2:\n" \
- ".section .fixup,\"ax\"\n" \
- "3: li %0,%3\n" \
- " b 2b\n" \
- ".previous\n" \
- EX_TABLE(1b, 3b) \
- : "=r" (err) \
- : "r" (x), "m<>" (*addr), "i" (-EFAULT), "0" (err))
-
-#ifdef __powerpc64__
-#define __put_user_asm2(x, ptr, retval) \
- __put_user_asm(x, ptr, retval, "std")
-#else /* __powerpc64__ */
-#define __put_user_asm2(x, addr, err) \
- __asm__ __volatile__( \
- "1: stw%X2 %1,%2\n" \
- "2: stw%X2 %L1,%L2\n" \
- "3:\n" \
- ".section .fixup,\"ax\"\n" \
- "4: li %0,%3\n" \
- " b 3b\n" \
- ".previous\n" \
- EX_TABLE(1b, 4b) \
- EX_TABLE(2b, 4b) \
- : "=r" (err) \
- : "r" (x), "m" (*addr), "i" (-EFAULT), "0" (err))
-#endif /* __powerpc64__ */
-
#define __put_user_size_allowed(x, ptr, size, retval) \
do { \
__label__ __pu_failed; \
@@ -249,6 +213,11 @@ do { \
})
+/*
+ * We don't tell gcc that we are accessing memory, but this is OK
+ * because we do not write to any memory gcc knows about, so there
+ * are no aliasing issues.
+ */
#define __put_user_asm_goto(x, addr, label, op) \
asm volatile goto( \
"1: " op "%U1%X1 %0,%1 # put_user\n" \
--
2.25.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox