Hello Ritesh, Please check my responses inline. - Gaurav On 7/29/26 8:35 PM, Ritesh Harjani (IBM) wrote: > Hi Gaurav, > > Gaurav Batra writes: > >> In powerPC, if Dynamic DMA Window is big enough, RAM is pre-mapped. To >> determine the size of RAM, a PAPR+ property "ibm,lrdr-capacity" is used. >> This OF property dictates what is the max size of RAM an LPAR can have, >> including DR added memory. >> >> In PowerPC, 16GB pages can be allocated at machine level and then >> assigned to LPARs. These 16GB pages are added to LPAR memory at the time >> of boot. The address range for these 16GB pages is above MAX RAM an LPAR >> can have (ibm,lrdr-capacity). In the current implementation, these 16GB > So these 16GB pages must be apperaing as "memory" nodes, because that's > how the code must be detecting them above "ibm,lrdr-capacity"? > Do you have the device node example for the same? yes, these 16GB pages appear as memory node right after the max-memory possible for the LPAR. Here are the properties from the device tree for a case where LPAR min/desired/max is 256GB/512GB/2048GB memory@20000000000: ibm,expected#pages                  00000000 ibm,phandle      80002000 device_type      "memory" reg              00000200 00000000 00000004 00000000 phandle          00cdd850 (13490256) name             "memory" ibm,associativity                  00000004 00000000 00000001 00000006 0000000c memory@20400000000: ibm,expected#pages                  00000000 ibm,phandle      80002040 device_type      "memory" reg              00000204 00000000 00000004 00000000 phandle          00cdda38 (13490744) name             "memory" ibm,associativity                  00000004 00000000 00000001 00000007 0000000e This LPAR has 2 16GB pages addigned to it and so, there are 2 16GB nodes presented to the LPAR. > > >> pages are being excluded from pre-mapped TCEs. A driver can have DMA >> buffers allocated from 16GB pages. This results in platform to raise an >> EEH when DMA is attempted on buffers in 16GB memory range. >> >> commit 6aa989ab2bd0 ("powerpc/pseries/iommu: memory notifier incorrectly >> adds TCEs for pmemory") >> >> Prior to the above patch, memblock_end_of_DRAM() was being used to >> determine the MAX memory of an LPAR. This included 16GB pages as well. >> The issue with using memblock_end_of_DRAM() is that when pmemory is >> converted to RAM via daxctl command, the DDW engine will incorrectly try >> to add TCEs for pmemory as well. >> >> Below is the address distribution of RAM, 16GB pages and pmemory for an >> LPAR with max memory of 256GB, memory allocated 64GB, 2 16GB pages and >> assigned pmemory of 8GB. >> >> RANGE SIZE STATE REMOVABLE BLOCK >> 0x0000000000000000-0x0000000fffffffff 64G online yes 0-255 >> 0x0000004000000000-0x00000047ffffffff 32G online yes 1024-1151 >> >> cat /sys/bus/nd/devices/region0/resource >> 0x40100000000 >> cat /sys/bus/nd/devices/region0/size >> 8589934592 > Is there any formal placement contract in the PAPR which tells where > would PMEM appear in the logical real address range for LPAR? I went through the PAPR for placement requirements for pmemory but, didn't see anything. I was able to get access to a machine which has 8TB of memory. I checked the placement of pmemory with various memory settings and 2 16GB of pages. Size of pmemory assigned to the LPAR is 8GB. mem     256/512/2048    (min/desired/max) [root@ltcmcka1n-lp1 ~]# lsmem RANGE                                  SIZE  STATE REMOVABLE  BLOCK 0x0000000000000000-0x0000007fffffffff  512G online       yes 0-2047 0x0000020000000000-0x00000207ffffffff   32G online       yes 8192-8319 [root@ltcmcka1n-lp1 ~]# cat /sys/bus/nd/devices/region0/resource 0x40010000000   <=====  pmemory placement above 4TB mem     256/512/4096 [root@ltcmcka1n-lp1 ~]# lsmem RANGE                                  SIZE  STATE REMOVABLE  BLOCK 0x0000000000000000-0x0000007fffffffff  512G online       yes 0-2047 0x0000040000000000-0x00000407ffffffff   32G online       yes 16384-16511 [root@ltcmcka1n-lp1 ~]# cat /sys/bus/nd/devices/region0/resource 0x80010000000   <===== 8TB mem     256/512/6144 [root@ltcmcka1n-lp1 ~]# lsmem RANGE                                  SIZE  STATE REMOVABLE  BLOCK 0x0000000000000000-0x0000007fffffffff  512G online       yes 0-2047 0x0000060000000000-0x00000607ffffffff   32G online       yes 24576-24703 [root@ltcmcka1n-lp1 ~]# cat /sys/bus/nd/devices/region0/resource 0x80010000000   <===== 8TB In all these cases, pmemory was assigned to above power-of-2 which included memory + 16GB pages. > >> The approach to fix this problem is to revert back the code changes >> introduced by the above patch and to stash away the MAX memory of an >> LPAR, including 16GB pages, at the LPAR boot time. This value is then >> used whenever TCEs are needed to be pre-mapped - enable_DDW() or, >> iommu_mem_notifier() > So, can it appear for e.g. in above case from 288G to 512G? In your > example we see pmem coming at 4TB, but what if the system DRAM was above > 4TB? In that case, could vPMEM appear at non-power-of-2 address just > above ibm,lrdr-capacity+16G? > > The reason for the ask is, I see a small window where we don't have any > direct pre-mappings, but we do allow for core dma mapping apis to use the > direct-mapping window. > > Here... > > In the current code we check for: > > @@ -2419,23 +2425,35 @@ static int iommu_mem_notifier(struct notifier_block *nb, unsigned long action, > { > + unsigned long max_ram_pages = pseries_ddw_max_ram >> PAGE_SHIFT; > int ret = 0; > <...> > + if (arg->start_pfn >= max_ram_pages) > + return NOTIFY_OK; > > & but in enable_ddw() { > <...> > int max_ram_len = order_base_2(pseries_ddw_max_ram); > <...> > /* For pre-mapped memory, set bus_dma_limit to the max RAM */ > if (direct_mapping) > dev->dev.bus_dma_limit = dev->dev.archdata.dma_offset + > (1ULL << max_ram_len); > > So, we don't pre-map anything above pseries_ddw_max_ram in the > iommu_notifier() but we allow for dma_mapping to use direct map until > order_base_2(pseries_ddw_max_ram). > So it looks like that, there is a small gap left in direct_mapping > window for e.g. in above case between 288G to 512G (because of > order_base_2()), where there are no pre-mapped TCE entries but if the > PMEM is added within that range, then we do allow to use the > direct-mapping path (due to bus_dma_limit) - which means an EEH could > occur. > > Could you please check this claim? Now, as per PAPR, when an OS is making an RTAS call to create DDW, the size of the DDW needs to be power-of-2. But, I didn't see any such requirement for the placement of pmemory. Even though, in my experiment above, pmemory was always given a region above power-of-2 of (RAM + 16GB), I think, we cannot assume that will be the case all the time. It could be different for different configurations. As you correctly pointed out in the enable_ddw() code, this could lead to EEH if pmemory is assigned to the same power-of-2 chunk as (RAM + 16GB pages). It would be better to set device bus_dma_limit to pseries_ddw_max_ram and *not* max_ram_pages, which in this case is the size of the DDW. I will fix this in my next version of the patch. Please let me know if you have any other concerns. I will incorporate all those changes in the next version of the patch. Thanks a lot for spending time and checking this code in deep. > > -ritesh > > >> Fixes: 6aa989ab2bd0 ("powerpc/pseries/iommu: memory notifier incorrectly adds TCEs for pmemory") >> Cc:stable@vger.kernel.org >> Signed-off-by: Gaurav Batra >> --- >> >> Change log: >> >> V3 -> V4 >> >> 1. Ritesh: Change pseries_ddw_max_ram to __ro_after_init; >> >> Response: Incorporated changes >> >> 2. Ritesh: Mark function ddw_memory_hotplug_max() as __init function >> >> Response: Incorporated changes >> >> 3. Ritesh: This patch was made in v6.15. Since we want this to be backported, I >> would suggested add a CC stable tag as well. >> >> Response: Incorporated changes >> >> V2 -> V3 >> >> 1. Harsh: Remove R-b tags from the change log >> >> Response: Incorporated changes >> >> 2. Harsh: Change WARN_ON() to WARN_ONCE() >> >> Response: Incorporated changes >> >> 3. Harsh: Fix indendation >> >> Response: Incorporated changes >> >> 4. Harsh: Replace comment with a log if limit < arg->nr_pages ? >> >> Response: Doesn't seems to be needed since the WARN_ONCE() will log this >> scenario. I removed the comment instead. >> >> V1 -> V2 >> >> 1. Harsh: Not only start_pfn, but end_pfn also needs to be within allowed >> range, which may require clamping arg->nr_pages if crossing the limits. >> >> Response: Incorporated changes. >> >> >> arch/powerpc/platforms/pseries/iommu.c | 60 ++++++++++++++++++-------- >> 1 file changed, 42 insertions(+), 18 deletions(-) >> >> diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c >> index 3e1f915fe4f6..7f63adafaf91 100644 >> --- a/arch/powerpc/platforms/pseries/iommu.c >> +++ b/arch/powerpc/platforms/pseries/iommu.c >> @@ -69,6 +69,8 @@ static struct iommu_table *iommu_pseries_alloc_table(int node) >> return tbl; >> } >> >> +static phys_addr_t pseries_ddw_max_ram __ro_after_init; >> + >> #ifdef CONFIG_IOMMU_API >> static struct iommu_table_group_ops spapr_tce_table_group_ops; >> #endif >> @@ -1283,15 +1285,19 @@ struct failed_ddw_pdn { >> >> static LIST_HEAD(failed_ddw_pdn_list); >> >> -static phys_addr_t ddw_memory_hotplug_max(void) >> +static phys_addr_t __init ddw_memory_hotplug_max(void) >> { >> - resource_size_t max_addr; >> + resource_size_t max_addr = memory_hotplug_max(); >> + struct device_node *memory; >> >> -#if defined(CONFIG_NUMA) && defined(CONFIG_MEMORY_HOTPLUG) >> - max_addr = hot_add_drconf_memory_max(); >> -#else >> - max_addr = memblock_end_of_DRAM(); >> -#endif >> + for_each_node_by_type(memory, "memory") { >> + struct resource res; >> + >> + if (of_address_to_resource(memory, 0, &res)) >> + continue; >> + >> + max_addr = max_t(resource_size_t, max_addr, res.end + 1); >> + } >> >> return max_addr; >> } >> @@ -1446,7 +1452,7 @@ static struct property *ddw_property_create(const char *propname, u32 liobn, u64 >> static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn, u64 dma_mask) >> { >> int len = 0, ret; >> - int max_ram_len = order_base_2(ddw_memory_hotplug_max()); >> + int max_ram_len = order_base_2(pseries_ddw_max_ram); >> struct ddw_query_response query; >> struct ddw_create_response create; >> int page_shift; >> @@ -1668,7 +1674,7 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn, u64 dma_mas >> >> if (direct_mapping) { >> /* DDW maps the whole partition, so enable direct DMA mapping */ >> - ret = walk_system_ram_range(0, ddw_memory_hotplug_max() >> PAGE_SHIFT, >> + ret = walk_system_ram_range(0, pseries_ddw_max_ram >> PAGE_SHIFT, >> win64->value, tce_setrange_multi_pSeriesLP_walk); >> if (ret) { >> dev_info(&dev->dev, "failed to map DMA window for %pOF: %d\n", >> @@ -2419,23 +2425,35 @@ static int iommu_mem_notifier(struct notifier_block *nb, unsigned long action, >> { >> struct dma_win *window; >> struct memory_notify *arg = data; >> + unsigned long limit = arg->nr_pages; >> + unsigned long max_ram_pages = pseries_ddw_max_ram >> PAGE_SHIFT; >> int ret = 0; >> >> /* This notifier can get called when onlining persistent memory as well. >> * TCEs are not pre-mapped for persistent memory. Persistent memory will >> - * always be above ddw_memory_hotplug_max() >> + * always be above pseries_ddw_max_ram >> */ >> + if (arg->start_pfn >= max_ram_pages) >> + return NOTIFY_OK; >> + >> + /* RAM is being DLPAR'ed. The range should never exceed max ram. >> + * Just in case, clamp the range and throw a warning. >> + */ >> + if (arg->start_pfn + limit > max_ram_pages) { >> + limit = max_ram_pages - arg->start_pfn; >> + WARN_ONCE(1, "Limiting Page Range %lx - %lx to Max Mem Pages: %lx\n", >> + arg->start_pfn, arg->start_pfn + arg->nr_pages, >> + max_ram_pages); >> + } >> >> switch (action) { >> case MEM_GOING_ONLINE: >> spin_lock(&dma_win_list_lock); >> list_for_each_entry(window, &dma_win_list, list) { >> - if (window->direct && (arg->start_pfn << PAGE_SHIFT) < >> - ddw_memory_hotplug_max()) { >> + if (window->direct) { >> ret |= tce_setrange_multi_pSeriesLP(arg->start_pfn, >> - arg->nr_pages, window->prop); >> + limit, window->prop); >> } >> - /* XXX log error */ >> } >> spin_unlock(&dma_win_list_lock); >> break; >> @@ -2443,12 +2461,10 @@ static int iommu_mem_notifier(struct notifier_block *nb, unsigned long action, >> case MEM_OFFLINE: >> spin_lock(&dma_win_list_lock); >> list_for_each_entry(window, &dma_win_list, list) { >> - if (window->direct && (arg->start_pfn << PAGE_SHIFT) < >> - ddw_memory_hotplug_max()) { >> + if (window->direct) { >> ret |= tce_clearrange_multi_pSeriesLP(arg->start_pfn, >> - arg->nr_pages, window->prop); >> + limit, window->prop); >> } >> - /* XXX log error */ >> } >> spin_unlock(&dma_win_list_lock); >> break; >> @@ -2532,6 +2548,14 @@ void __init iommu_init_early_pSeries(void) >> register_memory_notifier(&iommu_mem_nb); >> >> set_pci_dma_ops(&dma_iommu_ops); >> + >> + /* During init determine the max memory an LPAR can have and set it. This >> + * will be used for pre-mapping RAM in DDW. memblock_end_of_DRAM() can >> + * change during the running of LPAR - daxctl can add pmemory as >> + * "system-ram". This memory range should not be pre-mapped in DDW since >> + * the address of pmemory can be much higher than the DDW size. >> + */ >> + pseries_ddw_max_ram = ddw_memory_hotplug_max(); >> } >> >> static int __init disable_multitce(char *str) >> >> base-commit: 6d35786de28116ecf78797a62b84e6bf3c45aa5a >> -- >> 2.39.3