May 2021 - Linuxarm - mailweb.openeuler.org

[PATCH] dma-iommu: Add a check to avoid dereference null pointer in function iommu_dma_map_sg()
by chenxiang 21 May '21

21 May '21

From: Xiang Chen <chenxiang66(a)hisilicon.com> The issue is reported by tool TscanCode, and it is possible to deference null pointer when prev is NULL which is the initial value. Signed-off-by: Xiang Chen <chenxiang66(a)hisilicon.com> --- drivers/iommu/dma-iommu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c index 4cb63b2..88a4f34 100644 --- a/drivers/iommu/dma-iommu.c +++ b/drivers/iommu/dma-iommu.c @@ -1042,7 +1042,7 @@ static int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg, * iova_len == 0, thus we cannot dereference prev the first * time through here (i.e. before it has a meaningful value). */ - if (pad_len && pad_len < s_length - 1) { + if (prev && pad_len && pad_len < s_length - 1) { prev->length += pad_len; iova_len += pad_len; } -- 2.8.1

3 2

Re: [PATCH 6/9] tty: hvc_console: Fix coding style issues of block comments
by Johan Hovold 21 May '21

21 May '21

On Mon, May 17, 2021 at 02:37:10PM +0800, Xiaofei Tan wrote: > Fix coding style issues of block comments, reported by checkpatch.pl. > Besides, add a period at the end of the sentenses. > > Signed-off-by: Xiaofei Tan <tanxiaofei(a)huawei.com> > --- > drivers/tty/hvc/hvc_console.c | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c > index 39018e5..a61cdf0 100644 > --- a/drivers/tty/hvc/hvc_console.c > +++ b/drivers/tty/hvc/hvc_console.c > @@ -177,7 +177,8 @@ static void hvc_console_print(struct console *co, const char *b, > r = cons_ops[index]->put_chars(vtermnos[index], c, i); > if (r <= 0) { > /* throw away characters on error > - * but spin in case of -EAGAIN */ > + * but spin in case of -EAGAIN. > + */ How is this an improvement? First, the multi-line comment style is /* * ... */ Second, that sentence is not capitalised so why do add a period? Third, why are you sending checkpatch.pl cleanups for files outside of staging? Unless doing some real changes to the files in question as well this is mostly just churn and noise that makes it harder to backport fixes and do code forensics for no real gain. Greg may disagree, but I don't think we should be encouraging this kind of patches. Johan

3 7

[PATCH v3] topology: use bin_attribute to avoid buff overflow
by Tian Tao 21 May '21

21 May '21

Use bin_attribute to display thread_siblings/core_cpus/core_siblings, die_cpus/package_cpus/book_siblings/drawer_siblings,avoiding NR_CPUS too big to cause buff overflow. This patch is based on the following discussion. https://www.spinics.net/lists/linux-doc/msg95921.html Signed-off-by: Tian Tao <tiantao6(a)hisilicon.com> --- v2: rewrite the function name##_read v3: remove the temp buffer. --- drivers/base/topology.c | 55 +++++++++++++++++++++++++++++++------------------ 1 file changed, 35 insertions(+), 20 deletions(-) diff --git a/drivers/base/topology.c b/drivers/base/topology.c index 4d254fc..87bddd0 100644 --- a/drivers/base/topology.c +++ b/drivers/base/topology.c @@ -21,11 +21,16 @@ static ssize_t name##_show(struct device *dev, \ return sysfs_emit(buf, "%d\n", topology_##name(dev->id)); \ } -#define define_siblings_show_map(name, mask) \ -static ssize_t name##_show(struct device *dev, \ - struct device_attribute *attr, char *buf) \ -{ \ - return cpumap_print_to_pagebuf(false, buf, topology_##mask(dev->id));\ +#define define_siblings_read(name, mask) \ +static ssize_t name##_read(struct file *file, struct kobject *kobj, \ + struct bin_attribute *attr, char *buf, \ + loff_t off, size_t count) \ +{ \ + struct device *dev = kobj_to_dev(kobj); \ + if (off != 0) \ + return 0; \ + return scnprintf(buf, count, "%*pb\n", nr_cpu_ids, \ + cpumask_bits(topology_##mask(dev->id))); \ } #define define_siblings_show_list(name, mask) \ @@ -37,7 +42,7 @@ static ssize_t name##_list_show(struct device *dev, \ } #define define_siblings_show_func(name, mask) \ - define_siblings_show_map(name, mask); \ + define_siblings_read(name, mask); \ define_siblings_show_list(name, mask) define_id_show_func(physical_package_id); @@ -50,30 +55,30 @@ define_id_show_func(core_id); static DEVICE_ATTR_RO(core_id); define_siblings_show_func(thread_siblings, sibling_cpumask); -static DEVICE_ATTR_RO(thread_siblings); +static BIN_ATTR_RO(thread_siblings, 0); static DEVICE_ATTR_RO(thread_siblings_list); define_siblings_show_func(core_cpus, sibling_cpumask); -static DEVICE_ATTR_RO(core_cpus); +static BIN_ATTR_RO(core_cpus, 0); static DEVICE_ATTR_RO(core_cpus_list); define_siblings_show_func(core_siblings, core_cpumask); -static DEVICE_ATTR_RO(core_siblings); +static BIN_ATTR_RO(core_siblings, 0); static DEVICE_ATTR_RO(core_siblings_list); define_siblings_show_func(die_cpus, die_cpumask); -static DEVICE_ATTR_RO(die_cpus); +static BIN_ATTR_RO(die_cpus, 0); static DEVICE_ATTR_RO(die_cpus_list); define_siblings_show_func(package_cpus, core_cpumask); -static DEVICE_ATTR_RO(package_cpus); +static BIN_ATTR_RO(package_cpus, 0); static DEVICE_ATTR_RO(package_cpus_list); #ifdef CONFIG_SCHED_BOOK define_id_show_func(book_id); static DEVICE_ATTR_RO(book_id); define_siblings_show_func(book_siblings, book_cpumask); -static DEVICE_ATTR_RO(book_siblings); +static BIN_ATTR_RO(book_siblings, 0); static DEVICE_ATTR_RO(book_siblings_list); #endif @@ -81,32 +86,41 @@ static DEVICE_ATTR_RO(book_siblings_list); define_id_show_func(drawer_id); static DEVICE_ATTR_RO(drawer_id); define_siblings_show_func(drawer_siblings, drawer_cpumask); -static DEVICE_ATTR_RO(drawer_siblings); +static BIN_ATTR_RO(drawer_siblings, 0; static DEVICE_ATTR_RO(drawer_siblings_list); #endif + +static struct bin_attribute *bin_attrs[] = { + &bin_attr_thread_siblings, + &bin_attr_core_cpus, + &bin_attr_core_siblings, + &bin_attr_die_cpus, + &bin_attr_package_cpus, +#ifdef CONFIG_SCHED_BOOK + &bin_attr_book_siblings, +#endif +#ifdef CONFIG_SCHED_DRAWER + &bin_attr_drawer_siblings, +#endif + NULL, +}; + static struct attribute *default_attrs[] = { &dev_attr_physical_package_id.attr, &dev_attr_die_id.attr, &dev_attr_core_id.attr, - &dev_attr_thread_siblings.attr, &dev_attr_thread_siblings_list.attr, - &dev_attr_core_cpus.attr, &dev_attr_core_cpus_list.attr, - &dev_attr_core_siblings.attr, &dev_attr_core_siblings_list.attr, - &dev_attr_die_cpus.attr, &dev_attr_die_cpus_list.attr, - &dev_attr_package_cpus.attr, &dev_attr_package_cpus_list.attr, #ifdef CONFIG_SCHED_BOOK &dev_attr_book_id.attr, - &dev_attr_book_siblings.attr, &dev_attr_book_siblings_list.attr, #endif #ifdef CONFIG_SCHED_DRAWER &dev_attr_drawer_id.attr, - &dev_attr_drawer_siblings.attr, &dev_attr_drawer_siblings_list.attr, #endif NULL @@ -114,6 +128,7 @@ static struct attribute *default_attrs[] = { static const struct attribute_group topology_attr_group = { .attrs = default_attrs, + .bin_attrs = bin_attrs, .name = "topology" }; -- 2.7.4

1 0

[PATCH RESEND] tty: hvc_console: Remove the repeated words 'no' and 'from'
by Xiaofei Tan 21 May '21

21 May '21

Remove the repeated words 'no' and 'from', reported by checkpatch.pl. Signed-off-by: Xiaofei Tan <tanxiaofei(a)huawei.com> --- drivers/tty/hvc/hvc_console.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c index cdcc64e..bdd8d2b 100644 --- a/drivers/tty/hvc/hvc_console.c +++ b/drivers/tty/hvc/hvc_console.c @@ -292,7 +292,7 @@ int hvc_instantiate(uint32_t vtermno, int index, const struct hv_ops *ops) if (vtermnos[index] != -1) return -1; - /* make sure no no tty has been registered in this index */ + /* make sure no tty has been registered in this index */ hp = hvc_get_by_index(index); if (hp) { tty_port_put(&hp->port); @@ -620,7 +620,7 @@ static u32 timeout = MIN_TIMEOUT; /* * Maximum number of bytes to get from the console driver if hvc_poll is * called from driver (and can't sleep). Any more than this and we break - * and start polling with khvcd. This value was derived from from an OpenBMC + * and start polling with khvcd. This value was derived from an OpenBMC * console with the OPAL driver that results in about 0.25ms interrupts off * latency. */ -- 2.8.1

1 0

Re: [PATCH 0/9] tty: hvc_console: Fix some coding style issues
by Greg KH 21 May '21

21 May '21

On Mon, May 17, 2021 at 02:37:04PM +0800, Xiaofei Tan wrote: > Fix some issues reported by checkpatch.pl. All of them are > coding style issues, no function changes. > > Xiaofei Tan (9): > tty: hvc_console: Fix spaces required around that '=' > tty: hvc_console: Fix "foo * bar" should be "foo *bar" > tty: hvc_console: Remove trailing whitespace > tty: hvc_console: Fix issues of code indent should use tabs > tty: hvc_console: Delete spaces prohibited around open parenthesis '(' > and ')' > tty: hvc_console: Fix coding style issues of block comments > tty: hvc_console: Add a blank line after declarations > tty: hvc_console: Remove the repeated words 'no' and 'from' > tty: hvc_console: Move open brace { on the previous line > > drivers/tty/hvc/hvc_console.c | 37 ++++++++++++++++++++++--------------- > 1 file changed, 22 insertions(+), 15 deletions(-) Do you use this driver? If so, great, I'm sure there are other "real" issues in it that need some work. But as Johan points out, doing drive-by checkpatch cleanups on random files that you do not use, isn't the best thing to do. If you just want to do this type of work, please do so in drivers/staging/ as it is most welcome there if you wish to get involved in kernel work to get experience before doing "real" stuff. thanks, g reg k-h

2 1

[PATCH 0/5] Use pm_runtime_resume_and_get()
by Tian Tao 20 May '21

20 May '21

patch#1,#2,#3,#4,#5 use pm_runtime_resume_and_get() to replace pm_runtime_get_sync and pm_runtime_put_noidle. This change is just to simplify the code, no actual functional changes. Tian Tao (5): spi: Use pm_runtime_resume_and_get() to replace open coding spi: spi-mem: Use pm_runtime_resume_and_get() to replace open coding spi: imx: Use pm_runtime_resume_and_get() to replace open coding spi: spi-ti-qspi: Use pm_runtime_resume_and_get() to replace open coding spi: sprd: Use pm_runtime_resume_and_get() to replace open coding drivers/spi/spi-imx.c | 6 ++---- drivers/spi/spi-mem.c | 3 +-- drivers/spi/spi-sprd.c | 3 +-- drivers/spi/spi-ti-qspi.c | 3 +-- drivers/spi/spi.c | 9 +++------ 5 files changed, 8 insertions(+), 16 deletions(-) -- 2.7.4

2 9

[PATCH] spi: Use pm_runtime_resume_and_get() to replace open coding
by Tian Tao 20 May '21

20 May '21

Found using coccicheck script under review at: https://lore.kernel.org/lkml/20210427141946.2478411-1-Julia.Lawall@inria.fr/ Signed-off-by: Tian Tao <tiantao6(a)hisilicon.com> --- drivers/spi/spi.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 57120de..2664b4a 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -1486,9 +1486,8 @@ static void __spi_pump_messages(struct spi_controller *ctlr, bool in_kthread) mutex_lock(&ctlr->io_mutex); if (!was_busy && ctlr->auto_runtime_pm) { - ret = pm_runtime_get_sync(ctlr->dev.parent); + ret = pm_runtime_resume_and_get(ctlr->dev.parent); if (ret < 0) { - pm_runtime_put_noidle(ctlr->dev.parent); dev_err(&ctlr->dev, "Failed to power device: %d\n", ret); mutex_unlock(&ctlr->io_mutex); @@ -3426,10 +3425,9 @@ int spi_setup(struct spi_device *spi) } if (spi->controller->auto_runtime_pm && spi->controller->set_cs) { - status = pm_runtime_get_sync(spi->controller->dev.parent); + status = pm_runtime_resume_and_get(spi->controller->dev.parent); if (status < 0) { mutex_unlock(&spi->controller->io_mutex); - pm_runtime_put_noidle(spi->controller->dev.parent); dev_err(&spi->controller->dev, "Failed to power device: %d\n", status); return status; @@ -3491,10 +3489,9 @@ int spi_set_cs_timing(struct spi_device *spi, struct spi_delay *setup, mutex_lock(&spi->controller->io_mutex); if (spi->controller->auto_runtime_pm) { - status = pm_runtime_get_sync(parent); + status = pm_runtime_resume_and_get(parent); if (status < 0) { mutex_unlock(&spi->controller->io_mutex); - pm_runtime_put_noidle(parent); dev_err(&spi->controller->dev, "Failed to power device: %d\n", status); return status; -- 2.7.4

2 1

[RFC 1/1] kernfs: Remove the limit of pagesize in the read of sysfs
by Tian Tao 20 May '21

20 May '21

Currently we use sysfs to display cpu topology, but when the number of cpus is very large, it may exceed a pagesize, so here we remove the pagesize limit from the read function first. Signed-off-by: Tian Tao <tiantao6(a)hisilicon.com> --- fs/kernfs/file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c index c757193..da25e94 100644 --- a/fs/kernfs/file.c +++ b/fs/kernfs/file.c @@ -184,7 +184,7 @@ static const struct seq_operations kernfs_seq_ops = { static ssize_t kernfs_file_read_iter(struct kiocb *iocb, struct iov_iter *iter) { struct kernfs_open_file *of = kernfs_of(iocb->ki_filp); - ssize_t len = min_t(size_t, iov_iter_count(iter), PAGE_SIZE); + ssize_t len = iov_iter_count(iter); const struct kernfs_ops *ops; char *buf; -- 2.7.4

1 0

[PATCH v2] topology: use bin_attribute to avoid buff overflow
by Tian Tao 19 May '21

19 May '21

Use bin_attribute to display thread_siblings/core_cpus/core_siblings, die_cpus/package_cpus/book_siblings/drawer_siblings,avoiding NR_CPUS too big to cause buff overflow. This patch is based on the following discussion. https://www.spinics.net/lists/linux-doc/msg95921.html Signed-off-by: Tian Tao <tiantao6(a)hisilicon.com> --- v2: rewrite the function name##_read --- drivers/base/topology.c | 57 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 37 insertions(+), 20 deletions(-) diff --git a/drivers/base/topology.c b/drivers/base/topology.c index 4d254fc..1b74ff5 100644 --- a/drivers/base/topology.c +++ b/drivers/base/topology.c @@ -14,6 +14,8 @@ #include <linux/hardirq.h> #include <linux/topology.h> +#define BUFF_SIZE ((NR_CPUS/32)*9) + #define define_id_show_func(name) \ static ssize_t name##_show(struct device *dev, \ struct device_attribute *attr, char *buf) \ @@ -21,11 +23,16 @@ static ssize_t name##_show(struct device *dev, \ return sysfs_emit(buf, "%d\n", topology_##name(dev->id)); \ } -#define define_siblings_show_map(name, mask) \ -static ssize_t name##_show(struct device *dev, \ - struct device_attribute *attr, char *buf) \ -{ \ - return cpumap_print_to_pagebuf(false, buf, topology_##mask(dev->id));\ +#define define_siblings_read(name, mask) \ +static ssize_t name##_read(struct file *file, struct kobject *kobj, \ + struct bin_attribute *attr, char *buf, \ + loff_t off, size_t count) \ +{ \ + struct device *dev = kobj_to_dev(kobj); \ + char temp[BUFF_SIZE] = {0}; \ + scnprintf(temp, BUFF_SIZE, "%*pb\n", nr_cpu_ids, \ + cpumask_bits(topology_##mask(dev->id))); \ + return memory_read_from_buffer(buf, count, &off, temp, BUFF_SIZE); \ } #define define_siblings_show_list(name, mask) \ @@ -37,7 +44,7 @@ static ssize_t name##_list_show(struct device *dev, \ } #define define_siblings_show_func(name, mask) \ - define_siblings_show_map(name, mask); \ + define_siblings_read(name, mask); \ define_siblings_show_list(name, mask) define_id_show_func(physical_package_id); @@ -50,30 +57,30 @@ define_id_show_func(core_id); static DEVICE_ATTR_RO(core_id); define_siblings_show_func(thread_siblings, sibling_cpumask); -static DEVICE_ATTR_RO(thread_siblings); +static BIN_ATTR_RO(thread_siblings, 0); static DEVICE_ATTR_RO(thread_siblings_list); define_siblings_show_func(core_cpus, sibling_cpumask); -static DEVICE_ATTR_RO(core_cpus); +static BIN_ATTR_RO(core_cpus, 0); static DEVICE_ATTR_RO(core_cpus_list); define_siblings_show_func(core_siblings, core_cpumask); -static DEVICE_ATTR_RO(core_siblings); +static BIN_ATTR_RO(core_siblings, 0); static DEVICE_ATTR_RO(core_siblings_list); define_siblings_show_func(die_cpus, die_cpumask); -static DEVICE_ATTR_RO(die_cpus); +static BIN_ATTR_RO(die_cpus, 0); static DEVICE_ATTR_RO(die_cpus_list); define_siblings_show_func(package_cpus, core_cpumask); -static DEVICE_ATTR_RO(package_cpus); +static BIN_ATTR_RO(package_cpus, 0); static DEVICE_ATTR_RO(package_cpus_list); #ifdef CONFIG_SCHED_BOOK define_id_show_func(book_id); static DEVICE_ATTR_RO(book_id); define_siblings_show_func(book_siblings, book_cpumask); -static DEVICE_ATTR_RO(book_siblings); +static BIN_ATTR_RO(book_siblings, 0); static DEVICE_ATTR_RO(book_siblings_list); #endif @@ -81,32 +88,41 @@ static DEVICE_ATTR_RO(book_siblings_list); define_id_show_func(drawer_id); static DEVICE_ATTR_RO(drawer_id); define_siblings_show_func(drawer_siblings, drawer_cpumask); -static DEVICE_ATTR_RO(drawer_siblings); +static BIN_ATTR_RO(drawer_siblings, 0); static DEVICE_ATTR_RO(drawer_siblings_list); #endif + +static struct bin_attribute *bin_attrs[] = { + &bin_attr_thread_siblings, + &bin_attr_core_cpus, + &bin_attr_core_siblings, + &bin_attr_die_cpus, + &bin_attr_package_cpus, +#ifdef CONFIG_SCHED_BOOK + &bin_attr_book_siblings, +#endif +#ifdef CONFIG_SCHED_DRAWER + &bin_attr_drawer_siblings, +#endif + NULL, +}; + static struct attribute *default_attrs[] = { &dev_attr_physical_package_id.attr, &dev_attr_die_id.attr, &dev_attr_core_id.attr, - &dev_attr_thread_siblings.attr, &dev_attr_thread_siblings_list.attr, - &dev_attr_core_cpus.attr, &dev_attr_core_cpus_list.attr, - &dev_attr_core_siblings.attr, &dev_attr_core_siblings_list.attr, - &dev_attr_die_cpus.attr, &dev_attr_die_cpus_list.attr, - &dev_attr_package_cpus.attr, &dev_attr_package_cpus_list.attr, #ifdef CONFIG_SCHED_BOOK &dev_attr_book_id.attr, - &dev_attr_book_siblings.attr, &dev_attr_book_siblings_list.attr, #endif #ifdef CONFIG_SCHED_DRAWER &dev_attr_drawer_id.attr, - &dev_attr_drawer_siblings.attr, &dev_attr_drawer_siblings_list.attr, #endif NULL @@ -114,6 +130,7 @@ static struct attribute *default_attrs[] = { static const struct attribute_group topology_attr_group = { .attrs = default_attrs, + .bin_attrs = bin_attrs, .name = "topology" }; -- 2.7.4

2 1

Re: [RFC PATCH v2 5/6] hw/arm/virt-acpi-build: Add PPTT table
by Salil Mehta 19 May '21

19 May '21

> From: Qemu-arm [mailto:qemu-arm-bounces+salil.mehta=huawei.com@nongnu.org] > On Behalf Of wangyanan (Y) > Sent: Thursday, May 13, 2021 6:10 AM > > Hi Drew, > > I got a question below, and hope your reply. Thanks! > On 2021/4/13 16:07, Yanan Wang wrote: > > Add the Processor Properties Topology Table (PPTT) to present > > CPU topology information to ACPI guests. Note, while a DT boot > > Linux guest with a non-flat CPU topology will see socket and > > core IDs being sequential integers starting from zero, e.g. > > with -smp 4,sockets=2,cores=2,threads=1 > > > > a DT boot produces > > > > cpu: 0 package_id: 0 core_id: 0 > > cpu: 1 package_id: 0 core_id: 1 > > cpu: 2 package_id: 1 core_id: 0 > > cpu: 3 package_id: 1 core_id: 1 > > > > an ACPI boot produces > > > > cpu: 0 package_id: 36 core_id: 0 > > cpu: 1 package_id: 36 core_id: 1 > > cpu: 2 package_id: 96 core_id: 2 > > cpu: 3 package_id: 96 core_id: 3 > > > > This is due to several reasons: > > > > 1) DT cpu nodes do not have an equivalent field to what the PPTT > > ACPI Processor ID must be, i.e. something equal to the MADT CPU > > UID or equal to the UID of an ACPI processor container. In both > > ACPI cases those are platform dependant IDs assigned by the > > vendor. > > > > 2) While QEMU is the vendor for a guest, if the topology specifies > > SMT (> 1 thread), then, with ACPI, it is impossible to assign a > > core-id the same value as a package-id, thus it is not possible > > to have package-id=0 and core-id=0. This is because package and > > core containers must be in the same ACPI namespace and therefore > > must have unique UIDs. > > > > 3) ACPI processor containers are not required for PPTT tables to > > be used and, due to the limitations of which IDs are selected > > described above in (2), they are not helpful for QEMU, so we > > don't build them with this patch. In the absence of them, Linux > > assigns its own unique IDs. The maintainers have chosen not to use > > counters from zero, but rather ACPI table offsets, which explains > > why the numbers are so much larger than with DT. > > > > 4) When there is no SMT (threads=1) the core IDs for ACPI boot guests > > match the logical CPU IDs, because these IDs must be equal to the > > MADT CPU UID (as no processor containers are present), and QEMU > > uses the logical CPU ID for these MADT IDs. > > > > Tested-by: Jiajie Li <lijiajie11(a)huawei.com> > > Signed-off-by: Andrew Jones <drjones(a)redhat.com> > > Signed-off-by: Ying Fang <fangying1(a)huawei.com> > > Signed-off-by: Yanan Wang <wangyanan55(a)huawei.com> > > --- > > hw/arm/virt-acpi-build.c | 63 ++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 63 insertions(+) > > > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > > index 2ad5dad1bf..03fd812d5a 100644 > > --- a/hw/arm/virt-acpi-build.c > > +++ b/hw/arm/virt-acpi-build.c > > @@ -436,6 +436,64 @@ build_srat(GArray *table_data, BIOSLinker *linker, > VirtMachineState *vms) > > vms->oem_table_id); > > } > > > > +/* PPTT */ > > +static void > > +build_pptt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > > +{ > > + int pptt_start = table_data->len; > > + int uid = 0, cpus = 0, socket = 0; > > + MachineState *ms = MACHINE(vms); > > + unsigned int smp_cores = ms->smp.cores; > > + unsigned int smp_threads = ms->smp.threads; > > + > > + acpi_data_push(table_data, sizeof(AcpiTableHeader)); > > + > > + for (socket = 0; cpus < ms->possible_cpus->len; socket++) { > > + uint32_t socket_offset = table_data->len - pptt_start; > > + int core; > > + > > + build_processor_hierarchy_node( > > + table_data, 1, /* Physical package */ > > + 0, socket, /* No parent */ > > + NULL, 0); /* No private resources */ > > + > > + for (core = 0; core < smp_cores; core++) { > > + uint32_t core_offset = table_data->len - pptt_start; > > + int thread; > > + > > + if (smp_threads <= 1) { > > + build_processor_hierarchy_node( > > + table_data, > > + (1 << 1) | /* ACPI Processor ID valid */ > > + (1 << 3), /* ACPI 6.3 - Node is a Leaf */ > > + socket_offset, uid++, /* Parent is a Socket */ > > + NULL, 0); /* No private resources */ > > + } else { > > + build_processor_hierarchy_node( > > + table_data, 0, > > + socket_offset, core, /* Parent is a Socket */ > > + NULL, 0); /* No private resources */ > > + > > + for (thread = 0; thread < smp_threads; thread++) { > > + build_processor_hierarchy_node( > > + table_data, > > + (1 << 1) | /* ACPI Processor ID valid */ > > + (1 << 2) | /* ACPI 6.3 - Processor is a Thread */ > > + (1 << 3), /* ACPI 6.3 - Node is a Leaf */ > > + core_offset, uid++, /* Parent is a Core */ > > + NULL, 0); /* No private resources */ > > + } > > + } > > + } > > + cpus += smp_cores * smp_threads; > > + } > > + > > + build_header(linker, table_data, > > + (void *)(table_data->data + pptt_start), "PPTT", > > + table_data->len - pptt_start, 2, > > + vms->oem_id, vms->oem_table_id); > > +} > > + > > /* GTDT */ > > static void > > build_gtdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > > @@ -707,6 +765,11 @@ void virt_acpi_build(VirtMachineState *vms, > AcpiBuildTables *tables) > > acpi_add_table(table_offsets, tables_blob); > > build_madt(tables_blob, tables->linker, vms); > > > > + if (ms->smp.cpus > 1 && !vmc->no_cpu_topology) { > I'm not really sure why we need to care about "ms->smp.cpus > 1" here? > > IMO, just like MADT in which we create both ENABLED and DISABLED > gicc nodes no matter of number of ENABLED nodes is one or not, we > should create PPTT table for all the possible cpus and not care about > number of smp cpus, too. This will be more consistent with the ACPI > specification and the PPTT table will be used for ACPI cpu hotplug in > the future even with "smp.cpus == 1". A humble request: Let us not anticipate the changes of vcpu Hotplug here. Things are fluid with respect to the vcpu Hotplug right now and I think it will not be right to base PPTT Table changes in anticipation of something we are not sure of what it looks like. Any such decisions should be postponed and be made part of the actual vcpu Hotplug changes when(and if ever) they come for ARM64. This will also ensure proper review of such changes and useful in that particular context. Thanks > > Care of "smp.cpus > 1" in the DT cpu-map part makes sense to me, > because we are required to only add present cpu nodes to the DT and > Linux Doc says that a cpu-map is not needed for uniprocessor systems. > > Thanks, > Yanan > > + acpi_add_table(table_offsets, tables_blob); > > + build_pptt(tables_blob, tables->linker, vms); > > + } > > + > > acpi_add_table(table_offsets, tables_blob); > > build_gtdt(tables_blob, tables->linker, vms); > >

3 13