From 405de6e571a2bf332452a17ae98f7b3a0613365e Mon Sep 17 00:00:00 2001
From: Petr Berky petr.berky@email.cz Date: Tue, 14 Mar 2017 20:30:52 +0100 Subject: [PATCH] config: Add function to check if fw_cfg exists
It was found qemu_get_present_cpus_count may return impossible number of cpus because of not checking if fw_cfg exists before using it. That may lead to undefined behavior of emulator, in particular Bochs that freezes.
Signed-off-by: Petr Berky petr.berky@email.cz --- src/fw/paravirt.c | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-)
diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c index 707502d..b2cfc23 100644 --- a/src/fw/paravirt.c +++ b/src/fw/paravirt.c @@ -220,6 +220,21 @@ qemu_cfg_select(u16 f) outw(f, PORT_QEMU_CFG_CTL); }
+static int +qemu_cfg_check_signature(void) +{ + int i; + char *sig = "QEMU"; + + qemu_cfg_select(QEMU_CFG_SIGNATURE); + for (i = 0; i < 4; i++) { + if (inb(PORT_QEMU_CFG_DATA) != sig[i]) { + return -1; + } + } + return 0; +} + static void qemu_cfg_dma_transfer(void *address, u32 length, u32 control) { @@ -392,7 +407,9 @@ u16 qemu_get_present_cpus_count(void) { u16 smp_count = 0; - qemu_cfg_read_entry(&smp_count, QEMU_CFG_NB_CPUS, sizeof(smp_count)); + if (qemu_cfg_check_signature() == 0) { + qemu_cfg_read_entry(&smp_count, QEMU_CFG_NB_CPUS, sizeof(smp_count)); + } u16 cmos_cpu_count = rtc_read(CMOS_BIOS_SMP_COUNT) + 1; if (smp_count < cmos_cpu_count) { smp_count = cmos_cpu_count; @@ -563,12 +580,9 @@ void qemu_cfg_init(void) return;
// Detect fw_cfg interface. - qemu_cfg_select(QEMU_CFG_SIGNATURE); - char *sig = "QEMU"; - int i; - for (i = 0; i < 4; i++) - if (inb(PORT_QEMU_CFG_DATA) != sig[i]) - return; + if (qemu_cfg_check_signature() != 0) { + return; + }
dprintf(1, "Found QEMU fw_cfg\n");
On 03/14/17 21:33, petr.berky@email.cz wrote:
From 405de6e571a2bf332452a17ae98f7b3a0613365e Mon Sep 17 00:00:00 2001 From: Petr Berky petr.berky@email.cz Date: Tue, 14 Mar 2017 20:30:52 +0100 Subject: [PATCH] config: Add function to check if fw_cfg exists
It was found qemu_get_present_cpus_count may return impossible number of cpus because of not checking if fw_cfg exists before using it. That may lead to undefined behavior of emulator, in particular Bochs that freezes.
Signed-off-by: Petr Berky petr.berky@email.cz
src/fw/paravirt.c | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-)
diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c index 707502d..b2cfc23 100644 --- a/src/fw/paravirt.c +++ b/src/fw/paravirt.c @@ -220,6 +220,21 @@ qemu_cfg_select(u16 f) outw(f, PORT_QEMU_CFG_CTL); }
+static int +qemu_cfg_check_signature(void) +{
- int i;
- char *sig = "QEMU";
- qemu_cfg_select(QEMU_CFG_SIGNATURE);
- for (i = 0; i < 4; i++) {
if (inb(PORT_QEMU_CFG_DATA) != sig[i]) {return -1;}- }
- return 0;
+}
static void qemu_cfg_dma_transfer(void *address, u32 length, u32 control) { @@ -392,7 +407,9 @@ u16 qemu_get_present_cpus_count(void) { u16 smp_count = 0;
- qemu_cfg_read_entry(&smp_count, QEMU_CFG_NB_CPUS, sizeof(smp_count));
- if (qemu_cfg_check_signature() == 0) {
qemu_cfg_read_entry(&smp_count, QEMU_CFG_NB_CPUS, sizeof(smp_count));- } u16 cmos_cpu_count = rtc_read(CMOS_BIOS_SMP_COUNT) + 1; if (smp_count < cmos_cpu_count) { smp_count = cmos_cpu_count;
@@ -563,12 +580,9 @@ void qemu_cfg_init(void) return;
// Detect fw_cfg interface.
- qemu_cfg_select(QEMU_CFG_SIGNATURE);
- char *sig = "QEMU";
- int i;
- for (i = 0; i < 4; i++)
if (inb(PORT_QEMU_CFG_DATA) != sig[i])return;
if (qemu_cfg_check_signature() != 0) {
return;}
dprintf(1, "Found QEMU fw_cfg\n");
"src/fw/paravirt.c" already has an extern function called qemu_cfg_dma_enabled(), whic is based on the static global variable "cfg_dma_enabled", which is set in qemu_cfg_init().
The above is about the DMA interface for fw_cfg. I think it would be a "natural" extension to add a similar global variable and helper function (called qemu_cfg_enabled()) for the basic fw_cfg presence as well.
Then qemu_cfg_check_signature() would not be necessary in this patch.
Just an idea of course.
Laszlo
On 03/14/2017 09:46 PM, Laszlo Ersek wrote:
On 03/14/17 21:33, petr.berky@email.cz wrote:
From 405de6e571a2bf332452a17ae98f7b3a0613365e Mon Sep 17 00:00:00 2001 From: Petr Berky petr.berky@email.cz Date: Tue, 14 Mar 2017 20:30:52 +0100 Subject: [PATCH] config: Add function to check if fw_cfg exists
It was found qemu_get_present_cpus_count may return impossible number of cpus because of not checking if fw_cfg exists before using it. That may lead to undefined behavior of emulator, in particular Bochs that freezes.
Signed-off-by: Petr Berky petr.berky@email.cz
src/fw/paravirt.c | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-)
diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c index 707502d..b2cfc23 100644 --- a/src/fw/paravirt.c +++ b/src/fw/paravirt.c @@ -220,6 +220,21 @@ qemu_cfg_select(u16 f) outw(f, PORT_QEMU_CFG_CTL); }
+static int +qemu_cfg_check_signature(void) +{
- int i;
- char *sig = "QEMU";
- qemu_cfg_select(QEMU_CFG_SIGNATURE);
- for (i = 0; i < 4; i++) {
if (inb(PORT_QEMU_CFG_DATA) != sig[i]) {return -1;}- }
- return 0;
+}
static void qemu_cfg_dma_transfer(void *address, u32 length, u32 control) { @@ -392,7 +407,9 @@ u16 qemu_get_present_cpus_count(void) { u16 smp_count = 0;
- qemu_cfg_read_entry(&smp_count, QEMU_CFG_NB_CPUS, sizeof(smp_count));
- if (qemu_cfg_check_signature() == 0) {
qemu_cfg_read_entry(&smp_count, QEMU_CFG_NB_CPUS, sizeof(smp_count));- } u16 cmos_cpu_count = rtc_read(CMOS_BIOS_SMP_COUNT) + 1; if (smp_count < cmos_cpu_count) { smp_count = cmos_cpu_count;
@@ -563,12 +580,9 @@ void qemu_cfg_init(void) return;
// Detect fw_cfg interface.
- qemu_cfg_select(QEMU_CFG_SIGNATURE);
- char *sig = "QEMU";
- int i;
- for (i = 0; i < 4; i++)
if (inb(PORT_QEMU_CFG_DATA) != sig[i])return;
if (qemu_cfg_check_signature() != 0) {
return;}
dprintf(1, "Found QEMU fw_cfg\n");
"src/fw/paravirt.c" already has an extern function called qemu_cfg_dma_enabled(), whic is based on the static global variable "cfg_dma_enabled", which is set in qemu_cfg_init().
The above is about the DMA interface for fw_cfg. I think it would be a "natural" extension to add a similar global variable and helper function (called qemu_cfg_enabled()) for the basic fw_cfg presence as well.
Then qemu_cfg_check_signature() would not be necessary in this patch.
Just an idea of course.
Laszlo
I think you are right. I will prepare a second patch
From b06589c683a7defb4853a3b810bd7e6a12abe2d6 Mon Sep 17 00:00:00 2001 From: Petr Berky petr.berky@email.cz Date: Tue, 14 Mar 2017 23:32:15 +0100 Subject: [PATCH v2] config: Add function to check if fw_cfg exists
It was found qemu_get_present_cpus_count may return impossible number of cpus because of not checking if fw_cfg exists before using it. That may lead to undefined behavior of emulator, in particular Bochs that freezes.
Signed-off-by: Petr Berky petr.berky@email.cz --- src/fw/paravirt.c | 12 +++++++++++- src/fw/paravirt.h | 1 + 2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c index 707502d..dfc69d4 100644 --- a/src/fw/paravirt.c +++ b/src/fw/paravirt.c @@ -32,9 +32,16 @@ u32 RamSize; u64 RamSizeOver4G; // Type of emulator platform. int PlatformRunningOn VARFSEG; +// cfg enabled +int cfg_enabled = 0; // cfg_dma enabled int cfg_dma_enabled = 0;
+inline int qemu_cfg_enabled(void) +{ + return cfg_enabled; +} + inline int qemu_cfg_dma_enabled(void) { return cfg_dma_enabled; @@ -392,7 +399,9 @@ u16 qemu_get_present_cpus_count(void) { u16 smp_count = 0; - qemu_cfg_read_entry(&smp_count, QEMU_CFG_NB_CPUS, sizeof(smp_count)); + if (qemu_cfg_enabled()) { + qemu_cfg_read_entry(&smp_count, QEMU_CFG_NB_CPUS, sizeof(smp_count)); + } u16 cmos_cpu_count = rtc_read(CMOS_BIOS_SMP_COUNT) + 1; if (smp_count < cmos_cpu_count) { smp_count = cmos_cpu_count; @@ -570,6 +579,7 @@ void qemu_cfg_init(void) if (inb(PORT_QEMU_CFG_DATA) != sig[i]) return;
+ cfg_enabled = 1; dprintf(1, "Found QEMU fw_cfg\n");
// Detect DMA interface. diff --git a/src/fw/paravirt.h b/src/fw/paravirt.h index 16f3d9a..a14d83e 100644 --- a/src/fw/paravirt.h +++ b/src/fw/paravirt.h @@ -49,6 +49,7 @@ static inline int runningOnKVM(void) { // QEMU_CFG_DMA ID bit #define QEMU_CFG_VERSION_DMA 2
+int qemu_cfg_enabled(void); int qemu_cfg_dma_enabled(void); void qemu_preinit(void); void qemu_platform_setup(void);
On 03/15/17 00:09, Petr Berky wrote:
From b06589c683a7defb4853a3b810bd7e6a12abe2d6 Mon Sep 17 00:00:00 2001 From: Petr Berky petr.berky@email.cz Date: Tue, 14 Mar 2017 23:32:15 +0100 Subject: [PATCH v2] config: Add function to check if fw_cfg exists
It was found qemu_get_present_cpus_count may return impossible number of cpus because of not checking if fw_cfg exists before using it. That may lead to undefined behavior of emulator, in particular Bochs that freezes.
Signed-off-by: Petr Berky petr.berky@email.cz
src/fw/paravirt.c | 12 +++++++++++- src/fw/paravirt.h | 1 + 2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c index 707502d..dfc69d4 100644 --- a/src/fw/paravirt.c +++ b/src/fw/paravirt.c @@ -32,9 +32,16 @@ u32 RamSize; u64 RamSizeOver4G; // Type of emulator platform. int PlatformRunningOn VARFSEG; +// cfg enabled +int cfg_enabled = 0; // cfg_dma enabled int cfg_dma_enabled = 0;
+inline int qemu_cfg_enabled(void) +{
- return cfg_enabled;
+}
inline int qemu_cfg_dma_enabled(void) { return cfg_dma_enabled; @@ -392,7 +399,9 @@ u16 qemu_get_present_cpus_count(void) { u16 smp_count = 0;
- qemu_cfg_read_entry(&smp_count, QEMU_CFG_NB_CPUS, sizeof(smp_count));
- if (qemu_cfg_enabled()) {
qemu_cfg_read_entry(&smp_count, QEMU_CFG_NB_CPUS,sizeof(smp_count));
- } u16 cmos_cpu_count = rtc_read(CMOS_BIOS_SMP_COUNT) + 1; if (smp_count < cmos_cpu_count) { smp_count = cmos_cpu_count;
@@ -570,6 +579,7 @@ void qemu_cfg_init(void) if (inb(PORT_QEMU_CFG_DATA) != sig[i]) return;
cfg_enabled = 1; dprintf(1, "Found QEMU fw_cfg\n");
// Detect DMA interface.
If we wanted to parallel the DMA check 100%, we'd set the variable under the debug message, not above it, but even I am not that pedantic. :)
Reviewed-by: Laszlo Ersek lersek@redhat.com
Igor, can you check if this is safe for S3 resume too? I think it is, but I had better ask you.
Thanks Laszlo
diff --git a/src/fw/paravirt.h b/src/fw/paravirt.h index 16f3d9a..a14d83e 100644 --- a/src/fw/paravirt.h +++ b/src/fw/paravirt.h @@ -49,6 +49,7 @@ static inline int runningOnKVM(void) { // QEMU_CFG_DMA ID bit #define QEMU_CFG_VERSION_DMA 2
+int qemu_cfg_enabled(void); int qemu_cfg_dma_enabled(void); void qemu_preinit(void); void qemu_platform_setup(void);
On Wed, Mar 15, 2017 at 12:09:11AM +0100, Petr Berky wrote:
From b06589c683a7defb4853a3b810bd7e6a12abe2d6 Mon Sep 17 00:00:00 2001 From: Petr Berky petr.berky@email.cz Date: Tue, 14 Mar 2017 23:32:15 +0100 Subject: [PATCH v2] config: Add function to check if fw_cfg exists
It was found qemu_get_present_cpus_count may return impossible number of cpus because of not checking if fw_cfg exists before using it. That may lead to undefined behavior of emulator, in particular Bochs that freezes.
Thanks. The patch looks fine to me. However, it looks like it got whitespace corrupted. Can you retry with "git send-email", or if that's not applicable, try attaching the patch.
-Kevin
On 03/22/2017 04:34 PM, Kevin O'Connor wrote:
On Wed, Mar 15, 2017 at 12:09:11AM +0100, Petr Berky wrote:
From b06589c683a7defb4853a3b810bd7e6a12abe2d6 Mon Sep 17 00:00:00 2001 From: Petr Berky petr.berky@email.cz Date: Tue, 14 Mar 2017 23:32:15 +0100 Subject: [PATCH v2] config: Add function to check if fw_cfg exists
It was found qemu_get_present_cpus_count may return impossible number of cpus because of not checking if fw_cfg exists before using it. That may lead to undefined behavior of emulator, in particular Bochs that freezes.
Thanks. The patch looks fine to me. However, it looks like it got whitespace corrupted. Can you retry with "git send-email", or if that's not applicable, try attaching the patch.
-Kevin
I just send a patch v3 with this tiny change mentioned by Laszlo. I used "git send-email" as you advised and I hope you will receive it in the correct form this time.
-Petr
-
Kevin O'Connor -
Laszlo Ersek -
Petr Berky -
petr.berky＠email.cz