[PATCH 1/2] dm: remove ASL_COMPILER macro


Victor Sun
 

Previously ASL_COMPILER macro that indicates iasl location was passed from
host build machine environment, this makes no sense on target machine. If
user has a specific iasl path on host, then devicemodel will fail to run
on target machine.

The patch uses popen() to create a child process to get the exact iasl path
on target machine. If no iasl is found, devicemodel will exit directly.

Signed-off-by: Victor Sun <victor.sun@...>
---
devicemodel/core/main.c | 5 ++++
devicemodel/hw/platform/acpi/acpi.c | 39 ++++++++++++++++++++++++-----
devicemodel/include/acpi.h | 2 ++
3 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/devicemodel/core/main.c b/devicemodel/core/main.c
index 0e3d77bfa..6e6e2608e 100644
--- a/devicemodel/core/main.c
+++ b/devicemodel/core/main.c
@@ -855,6 +855,11 @@ main(int argc, char *argv[])
if (signal(SIGPIPE, SIG_IGN) == SIG_ERR)
fprintf(stderr, "cannot register handler for SIGPIPE\n");

+ if (get_iasl_compiler() != 0) {
+ pr_err("cannot find Intel ACPI ASL compiler tool \"iasl\"");
+ exit(1);
+ }
+
if (parse_madt()) {
pr_err("Failed to parse the MADT table\n");
exit(1);
diff --git a/devicemodel/hw/platform/acpi/acpi.c b/devicemodel/hw/platform/acpi/acpi.c
index 0ba25e879..6703b59d2 100644
--- a/devicemodel/hw/platform/acpi/acpi.c
+++ b/devicemodel/hw/platform/acpi/acpi.c
@@ -95,9 +95,8 @@

#define ASL_TEMPLATE "dm.XXXXXXX"
#define ASL_SUFFIX ".aml"
-#ifndef ASL_COMPILER
-#define ASL_COMPILER "/usr/sbin/iasl"
-#endif
+
+static char asl_compiler[MAXPATHLEN] = {0};

uint64_t audio_nhlt_len = 0;

@@ -972,7 +971,7 @@ basl_compile(struct vmctx *ctx,
uint64_t offset)
{
struct basl_fio io[2];
- static char iaslbuf[3*MAXPATHLEN + 10];
+ static char iaslbuf[4*MAXPATHLEN + 10];
int err;

err = basl_start(&io[0], &io[1]);
@@ -990,12 +989,12 @@ basl_compile(struct vmctx *ctx,
if (basl_verbose_iasl)
snprintf(iaslbuf, sizeof(iaslbuf),
"%s -p %s %s",
- ASL_COMPILER,
+ asl_compiler,
io[1].f_name, io[0].f_name);
else
snprintf(iaslbuf, sizeof(iaslbuf),
"/bin/sh -c \"%s -p %s %s\" 1> /dev/null",
- ASL_COMPILER,
+ asl_compiler,
io[1].f_name, io[0].f_name);

err = system(iaslbuf);
@@ -1110,6 +1109,34 @@ get_acpi_table_length(void)
return ACPI_LENGTH;
}

+int
+get_iasl_compiler(void)
+{
+ int ret = -1;
+ char c;
+ int i = 0;
+ FILE *fd_iasl = popen("which iasl", "r");
+
+ if (fd_iasl != NULL)
+ {
+ while (i < (MAXPATHLEN - 1)) {
+
+ c = fgetc(fd_iasl);
+ if ((c == EOF) || (c == 0x0d) || (c == 0x0a) || (c == 0)) {
+ break;
+ }
+
+ asl_compiler[i++] = c;
+ }
+ if (strlen(asl_compiler) > 0) {
+ pr_info("found iasl: %s\n", asl_compiler);
+ ret = 0;
+ }
+ pclose(fd_iasl);
+ }
+ return ret;
+}
+
int
acpi_build(struct vmctx *ctx, int ncpu)
{
diff --git a/devicemodel/include/acpi.h b/devicemodel/include/acpi.h
index 52130d220..e9941e677 100644
--- a/devicemodel/include/acpi.h
+++ b/devicemodel/include/acpi.h
@@ -121,4 +121,6 @@ int lapicid_from_pcpuid(int pcpu_id);
int lapic_to_pcpu(int lapic);

int parse_madt(void);
+int get_iasl_compiler(void);
+
#endif /* _ACPI_H_ */
--
2.25.1


Yu Wang
 

On Sun, Jun 12, 2022 at 01:42:02PM +0800, Victor Sun wrote:
Previously ASL_COMPILER macro that indicates iasl location was passed from
host build machine environment, this makes no sense on target machine. If
user has a specific iasl path on host, then devicemodel will fail to run
on target machine.

The patch uses popen() to create a child process to get the exact iasl path
on target machine. If no iasl is found, devicemodel will exit directly.

Signed-off-by: Victor Sun <victor.sun@...>
---
devicemodel/core/main.c | 5 ++++
devicemodel/hw/platform/acpi/acpi.c | 39 ++++++++++++++++++++++++-----
devicemodel/include/acpi.h | 2 ++
3 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/devicemodel/core/main.c b/devicemodel/core/main.c
index 0e3d77bfa..6e6e2608e 100644
--- a/devicemodel/core/main.c
+++ b/devicemodel/core/main.c
@@ -855,6 +855,11 @@ main(int argc, char *argv[])
if (signal(SIGPIPE, SIG_IGN) == SIG_ERR)
fprintf(stderr, "cannot register handler for SIGPIPE\n");

+ if (get_iasl_compiler() != 0) {
+ pr_err("cannot find Intel ACPI ASL compiler tool \"iasl\"");
+ exit(1);
+ }
+
if (parse_madt()) {
pr_err("Failed to parse the MADT table\n");
exit(1);
diff --git a/devicemodel/hw/platform/acpi/acpi.c b/devicemodel/hw/platform/acpi/acpi.c
index 0ba25e879..6703b59d2 100644
--- a/devicemodel/hw/platform/acpi/acpi.c
+++ b/devicemodel/hw/platform/acpi/acpi.c
@@ -95,9 +95,8 @@

#define ASL_TEMPLATE "dm.XXXXXXX"
#define ASL_SUFFIX ".aml"
-#ifndef ASL_COMPILER
-#define ASL_COMPILER "/usr/sbin/iasl"
-#endif
+
+static char asl_compiler[MAXPATHLEN] = {0};

uint64_t audio_nhlt_len = 0;

@@ -972,7 +971,7 @@ basl_compile(struct vmctx *ctx,
uint64_t offset)
{
struct basl_fio io[2];
- static char iaslbuf[3*MAXPATHLEN + 10];
+ static char iaslbuf[4*MAXPATHLEN + 10];
int err;

err = basl_start(&io[0], &io[1]);
@@ -990,12 +989,12 @@ basl_compile(struct vmctx *ctx,
if (basl_verbose_iasl)
snprintf(iaslbuf, sizeof(iaslbuf),
"%s -p %s %s",
- ASL_COMPILER,
+ asl_compiler,
io[1].f_name, io[0].f_name);
else
snprintf(iaslbuf, sizeof(iaslbuf),
"/bin/sh -c \"%s -p %s %s\" 1> /dev/null",
- ASL_COMPILER,
+ asl_compiler,
io[1].f_name, io[0].f_name);

err = system(iaslbuf);
@@ -1110,6 +1109,34 @@ get_acpi_table_length(void)
return ACPI_LENGTH;
}

+int
+get_iasl_compiler(void)
+{
+ int ret = -1;
+ char c;
+ int i = 0;
+ FILE *fd_iasl = popen("which iasl", "r");
+
+ if (fd_iasl != NULL)
+ {
+ while (i < (MAXPATHLEN - 1)) {
+
+ c = fgetc(fd_iasl);
+ if ((c == EOF) || (c == 0x0d) || (c == 0x0a) || (c == 0)) {
+ break;
+ }
What's mean of 0x0d, 0x0c?

+
+ asl_compiler[i++] = c;
+ }
If it is 0x0d or 0x0c, should we add '\0' at the end?

+ if (strlen(asl_compiler) > 0) {
+ pr_info("found iasl: %s\n", asl_compiler);
+ ret = 0;
+ }
+ pclose(fd_iasl);
+ }
+ return ret;
+}
+
int
acpi_build(struct vmctx *ctx, int ncpu)
{
diff --git a/devicemodel/include/acpi.h b/devicemodel/include/acpi.h
index 52130d220..e9941e677 100644
--- a/devicemodel/include/acpi.h
+++ b/devicemodel/include/acpi.h
@@ -121,4 +121,6 @@ int lapicid_from_pcpuid(int pcpu_id);
int lapic_to_pcpu(int lapic);

int parse_madt(void);
+int get_iasl_compiler(void);
+
#endif /* _ACPI_H_ */
--
2.25.1






Victor Sun
 

-----Original Message-----
From: acrn-dev@... <acrn-dev@...> On
Behalf Of Yu Wang
Sent: Monday, June 13, 2022 3:11 PM
To: acrn-dev@...
Subject: Re: [acrn-dev] [PATCH 1/2] dm: remove ASL_COMPILER macro

On Sun, Jun 12, 2022 at 01:42:02PM +0800, Victor Sun wrote:
Previously ASL_COMPILER macro that indicates iasl location was passed
from host build machine environment, this makes no sense on target
machine. If user has a specific iasl path on host, then devicemodel
will fail to run on target machine.

The patch uses popen() to create a child process to get the exact iasl
path on target machine. If no iasl is found, devicemodel will exit directly.

Signed-off-by: Victor Sun <victor.sun@...>
---
devicemodel/core/main.c | 5 ++++
devicemodel/hw/platform/acpi/acpi.c | 39 ++++++++++++++++++++++++---
--
devicemodel/include/acpi.h | 2 ++
3 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/devicemodel/core/main.c b/devicemodel/core/main.c index
0e3d77bfa..6e6e2608e 100644
--- a/devicemodel/core/main.c
+++ b/devicemodel/core/main.c
@@ -855,6 +855,11 @@ main(int argc, char *argv[])
if (signal(SIGPIPE, SIG_IGN) == SIG_ERR)
fprintf(stderr, "cannot register handler for SIGPIPE\n");

+ if (get_iasl_compiler() != 0) {
+ pr_err("cannot find Intel ACPI ASL compiler tool \"iasl\"");
+ exit(1);
+ }
+
if (parse_madt()) {
pr_err("Failed to parse the MADT table\n");
exit(1);
diff --git a/devicemodel/hw/platform/acpi/acpi.c
b/devicemodel/hw/platform/acpi/acpi.c
index 0ba25e879..6703b59d2 100644
--- a/devicemodel/hw/platform/acpi/acpi.c
+++ b/devicemodel/hw/platform/acpi/acpi.c
@@ -95,9 +95,8 @@

#define ASL_TEMPLATE "dm.XXXXXXX"
#define ASL_SUFFIX ".aml"
-#ifndef ASL_COMPILER
-#define ASL_COMPILER "/usr/sbin/iasl"
-#endif
+
+static char asl_compiler[MAXPATHLEN] = {0};

uint64_t audio_nhlt_len = 0;

@@ -972,7 +971,7 @@ basl_compile(struct vmctx *ctx,
uint64_t offset)
{
struct basl_fio io[2];
- static char iaslbuf[3*MAXPATHLEN + 10];
+ static char iaslbuf[4*MAXPATHLEN + 10];
int err;

err = basl_start(&io[0], &io[1]);
@@ -990,12 +989,12 @@ basl_compile(struct vmctx *ctx,
if (basl_verbose_iasl)
snprintf(iaslbuf, sizeof(iaslbuf),
"%s -p %s %s",
- ASL_COMPILER,
+ asl_compiler,
io[1].f_name, io[0].f_name);
else
snprintf(iaslbuf, sizeof(iaslbuf),
"/bin/sh -c \"%s -p %s %s\" 1>
/dev/null",
- ASL_COMPILER,
+ asl_compiler,
io[1].f_name, io[0].f_name);

err = system(iaslbuf);
@@ -1110,6 +1109,34 @@ get_acpi_table_length(void)
return ACPI_LENGTH;
}

+int
+get_iasl_compiler(void)
+{
+ int ret = -1;
+ char c;
+ int i = 0;
+ FILE *fd_iasl = popen("which iasl", "r");
+
+ if (fd_iasl != NULL)
+ {
+ while (i < (MAXPATHLEN - 1)) {
+
+ c = fgetc(fd_iasl);
+ if ((c == EOF) || (c == 0x0d) || (c == 0x0a) || (c == 0)) {
+ break;
+ }
What's mean of 0x0d, 0x0c?
Hi Yu:

Ascii 0x0d: '\r', 0x0a: '\n'.
How about change the code as below?
if ((c == EOF) || (c == '\n') || (c == '\r') || (c == 0)) {


+
+ asl_compiler[i++] = c;
+ }
If it is 0x0d or 0x0c, should we add '\0' at the end?
The asl_compiler[] has been initialized as all zero already:
static char asl_compiler[MAXPATHLEN] = {0};

BR,
Victor


+ if (strlen(asl_compiler) > 0) {
+ pr_info("found iasl: %s\n", asl_compiler);
+ ret = 0;
+ }
+ pclose(fd_iasl);
+ }
+ return ret;
+}
+
int
acpi_build(struct vmctx *ctx, int ncpu) { diff --git
a/devicemodel/include/acpi.h b/devicemodel/include/acpi.h index
52130d220..e9941e677 100644
--- a/devicemodel/include/acpi.h
+++ b/devicemodel/include/acpi.h
@@ -121,4 +121,6 @@ int lapicid_from_pcpuid(int pcpu_id); int
lapic_to_pcpu(int lapic);

int parse_madt(void);
+int get_iasl_compiler(void);
+
#endif /* _ACPI_H_ */
--
2.25.1








Yu Wang
 

On Mon, Jun 13, 2022 at 07:53:40AM +0000, Victor Sun wrote:


-----Original Message-----
From: acrn-dev@... <acrn-dev@...> On
Behalf Of Yu Wang
Sent: Monday, June 13, 2022 3:11 PM
To: acrn-dev@...
Subject: Re: [acrn-dev] [PATCH 1/2] dm: remove ASL_COMPILER macro

On Sun, Jun 12, 2022 at 01:42:02PM +0800, Victor Sun wrote:
Previously ASL_COMPILER macro that indicates iasl location was passed
from host build machine environment, this makes no sense on target
machine. If user has a specific iasl path on host, then devicemodel
will fail to run on target machine.

The patch uses popen() to create a child process to get the exact iasl
path on target machine. If no iasl is found, devicemodel will exit directly.

Signed-off-by: Victor Sun <victor.sun@...>
---
devicemodel/core/main.c | 5 ++++
devicemodel/hw/platform/acpi/acpi.c | 39 ++++++++++++++++++++++++---
--
devicemodel/include/acpi.h | 2 ++
3 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/devicemodel/core/main.c b/devicemodel/core/main.c index
0e3d77bfa..6e6e2608e 100644
--- a/devicemodel/core/main.c
+++ b/devicemodel/core/main.c
@@ -855,6 +855,11 @@ main(int argc, char *argv[])
if (signal(SIGPIPE, SIG_IGN) == SIG_ERR)
fprintf(stderr, "cannot register handler for SIGPIPE\n");

+ if (get_iasl_compiler() != 0) {
+ pr_err("cannot find Intel ACPI ASL compiler tool \"iasl\"");
Cannot

+ exit(1);
+ }
+
if (parse_madt()) {
pr_err("Failed to parse the MADT table\n");
exit(1);
diff --git a/devicemodel/hw/platform/acpi/acpi.c
b/devicemodel/hw/platform/acpi/acpi.c
index 0ba25e879..6703b59d2 100644
--- a/devicemodel/hw/platform/acpi/acpi.c
+++ b/devicemodel/hw/platform/acpi/acpi.c
@@ -95,9 +95,8 @@

#define ASL_TEMPLATE "dm.XXXXXXX"
#define ASL_SUFFIX ".aml"
-#ifndef ASL_COMPILER
-#define ASL_COMPILER "/usr/sbin/iasl"
-#endif
+
+static char asl_compiler[MAXPATHLEN] = {0};

uint64_t audio_nhlt_len = 0;

@@ -972,7 +971,7 @@ basl_compile(struct vmctx *ctx,
uint64_t offset)
{
struct basl_fio io[2];
- static char iaslbuf[3*MAXPATHLEN + 10];
+ static char iaslbuf[4*MAXPATHLEN + 10];
int err;

err = basl_start(&io[0], &io[1]);
@@ -990,12 +989,12 @@ basl_compile(struct vmctx *ctx,
if (basl_verbose_iasl)
snprintf(iaslbuf, sizeof(iaslbuf),
"%s -p %s %s",
- ASL_COMPILER,
+ asl_compiler,
io[1].f_name, io[0].f_name);
else
snprintf(iaslbuf, sizeof(iaslbuf),
"/bin/sh -c \"%s -p %s %s\" 1>
/dev/null",
- ASL_COMPILER,
+ asl_compiler,
io[1].f_name, io[0].f_name);

err = system(iaslbuf);
@@ -1110,6 +1109,34 @@ get_acpi_table_length(void)
return ACPI_LENGTH;
}

+int
+get_iasl_compiler(void)
+{
+ int ret = -1;
+ char c;
+ int i = 0;
+ FILE *fd_iasl = popen("which iasl", "r");
+
+ if (fd_iasl != NULL)
+ {
+ while (i < (MAXPATHLEN - 1)) {
+
+ c = fgetc(fd_iasl);
+ if ((c == EOF) || (c == 0x0d) || (c == 0x0a) || (c == 0)) {
+ break;
+ }
What's mean of 0x0d, 0x0c?
Hi Yu:

Ascii 0x0d: '\r', 0x0a: '\n'.
How about change the code as below?
if ((c == EOF) || (c == '\n') || (c == '\r') || (c == 0)) {
Yes. That's better.

Please PR with the fix.
Acked-by: Wang, Yu1 <yu1.wang@...>



+
+ asl_compiler[i++] = c;
+ }
If it is 0x0d or 0x0c, should we add '\0' at the end?
The asl_compiler[] has been initialized as all zero already:
static char asl_compiler[MAXPATHLEN] = {0};

BR,
Victor


+ if (strlen(asl_compiler) > 0) {
+ pr_info("found iasl: %s\n", asl_compiler);
+ ret = 0;
+ }
+ pclose(fd_iasl);
+ }
+ return ret;
+}
+
int
acpi_build(struct vmctx *ctx, int ncpu) { diff --git
a/devicemodel/include/acpi.h b/devicemodel/include/acpi.h index
52130d220..e9941e677 100644
--- a/devicemodel/include/acpi.h
+++ b/devicemodel/include/acpi.h
@@ -121,4 +121,6 @@ int lapicid_from_pcpuid(int pcpu_id); int
lapic_to_pcpu(int lapic);

int parse_madt(void);
+int get_iasl_compiler(void);
+
#endif /* _ACPI_H_ */
--
2.25.1