This patchset is part of the ongoing work to add virtio support to OpenBIOS, yet self-contained enough to be spun out as a separate patchset.
Signed-off-by: Mark Cave-Ayland mark.cave-ayland@ilande.co.uk
Mark Cave-Ayland (3): fw_cfg: split fw_cfg_read() into fw_cfg_read() and fw_cfg_read_bytes() fw_cfg: add fw_cfg_find_file() and fw_cfg_read_file() functions fw_cfg: implement fw-cfg-read-file Forth word
drivers/fw_cfg.c | 88 +++++++++++++++++++++++++++++++++++++++--- include/arch/common/fw_cfg.h | 16 ++++++++ libopenbios/init.c | 7 +++- 3 files changed, 104 insertions(+), 7 deletions(-)
This is in preparation for calling fw_cfg_read_bytes() separately when reading data from the fw_cfg interface.
Signed-off-by: Mark Cave-Ayland mark.cave-ayland@ilande.co.uk --- drivers/fw_cfg.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/fw_cfg.c b/drivers/fw_cfg.c index 4027570..7086873 100644 --- a/drivers/fw_cfg.c +++ b/drivers/fw_cfg.c @@ -9,26 +9,38 @@ static volatile uint16_t *fw_cfg_cmd; static volatile uint8_t *fw_cfg_data;
-void -fw_cfg_read(uint16_t cmd, char *buf, unsigned int nbytes) +static void +fw_cfg_read_bytes(char *buf, unsigned int nbytes) { unsigned int i;
- *fw_cfg_cmd = cmd; for (i = 0; i < nbytes; i++) buf[i] = *fw_cfg_data; } -#else -// XXX depends on PCI bus location, should be removed + void fw_cfg_read(uint16_t cmd, char *buf, unsigned int nbytes) { + *fw_cfg_cmd = cmd; + fw_cfg_read_bytes(buf, nbytes); +} +#else +// XXX depends on PCI bus location, should be removed +static void +fw_cfg_read_bytes(char *buf, unsigned int nbytes) +{ unsigned int i;
- outw(cmd, CONFIG_FW_CFG_ADDR); for (i = 0; i < nbytes; i++) buf[i] = inb(CONFIG_FW_CFG_ADDR + 1); } + +void +fw_cfg_read(uint16_t cmd, char *buf, unsigned int nbytes) +{ + outw(cmd, CONFIG_FW_CFG_ADDR); + fw_cfg_read_bytes(buf, nbytes); +} #endif
uint64_t
These allow OpenBIOS to locate fw_cfg files by name and then read them via the fw_cfg interface.
Signed-off-by: Mark Cave-Ayland mark.cave-ayland@ilande.co.uk --- drivers/fw_cfg.c | 41 +++++++++++++++++++++++++++++++++++++++++ include/arch/common/fw_cfg.h | 15 +++++++++++++++ 2 files changed, 56 insertions(+)
diff --git a/drivers/fw_cfg.c b/drivers/fw_cfg.c index 7086873..f930ea8 100644 --- a/drivers/fw_cfg.c +++ b/drivers/fw_cfg.c @@ -73,6 +73,47 @@ fw_cfg_read_i16(uint16_t cmd) return __le16_to_cpu(buf); }
+uint32_t +fw_cfg_find_file(const char *filename, uint16_t *select, uint32_t *size) +{ + FWCfgFile f; + unsigned int i; + uint32_t buf, count; + + fw_cfg_read(FW_CFG_FILE_DIR, (char *)&buf, sizeof(uint32_t)); + count = __be32_to_cpu(buf); + + for (i = 0; i < count; i++) { + fw_cfg_read_bytes((char *)&f, sizeof(f)); + + if (!strcmp(f.name, filename)) { + *select = f.select; + *size = f.size; + + return -1; + } + } + + return 0; +} + +char * +fw_cfg_read_file(const char *filename, uint32_t *size) +{ + uint16_t cmd; + uint32_t nbytes; + char *buf; + + if (fw_cfg_find_file(filename, &cmd, &nbytes)) { + buf = malloc(nbytes); + fw_cfg_read(cmd, buf, nbytes); + *size = nbytes; + return buf; + } + + return NULL; +} + void fw_cfg_init(void) { diff --git a/include/arch/common/fw_cfg.h b/include/arch/common/fw_cfg.h index df44c2e..1c43a9d 100644 --- a/include/arch/common/fw_cfg.h +++ b/include/arch/common/fw_cfg.h @@ -85,6 +85,21 @@ uint64_t fw_cfg_read_i64(uint16_t cmd); uint32_t fw_cfg_read_i32(uint16_t cmd); uint16_t fw_cfg_read_i16(uint16_t cmd); void fw_cfg_init(void); + +typedef struct FWCfgFile { + uint32_t size; /* file size */ + uint16_t select; /* write this to 0x510 to read it */ + uint16_t reserved; + char name[56]; +} FWCfgFile; + +typedef struct FWCfgFiles { + uint32_t count; + FWCfgFile f[]; +} FWCfgFiles; + +unsigned int fw_cfg_find_file(const char *filename, uint16_t *select, uint32_t *size); +char *fw_cfg_read_file(const char *filename, uint32_t *size); #endif /* NO_OPENBIOS_PROTOS */
#endif
Hi Mark,
On Sun, Mar 19, 2017 at 11:46:53AM +0000, Mark Cave-Ayland wrote:
- uint32_t buf, count;
- fw_cfg_read(FW_CFG_FILE_DIR, (char *)&buf, sizeof(uint32_t));
- count = __be32_to_cpu(buf);
It's a lot cleaner to use e.g.
count = fw_cfg_read32(FW_CFG_FILE_DIR);
and ideally you wouldn't use byte-swapping things at all:
uint32_t fw_cfg_read32(unsigned char *p) { uint32_t x = 0; for (int i = 0; i < 4; i++_ x = (x << 8) | p[i]; return x; }
Segher
On 19/03/17 19:04, Segher Boessenkool wrote:
Hi Mark,
On Sun, Mar 19, 2017 at 11:46:53AM +0000, Mark Cave-Ayland wrote:
- uint32_t buf, count;
- fw_cfg_read(FW_CFG_FILE_DIR, (char *)&buf, sizeof(uint32_t));
- count = __be32_to_cpu(buf);
It's a lot cleaner to use e.g.
count = fw_cfg_read32(FW_CFG_FILE_DIR);
Yeah I know what you mean here. The reason for the above was that the existing registers are LE, except for the fw_cfg API which is BE. And since this was the only BE I just added it straight in...
I'm not sure I'm too excited about splitting this into a separate function although I do see that I'm missing a couple of __be*_to_cpu() wrappers in this patch. I'll do a resend later.
and ideally you wouldn't use byte-swapping things at all:
uint32_t fw_cfg_read32(unsigned char *p) { uint32_t x = 0; for (int i = 0; i < 4; i++_ x = (x << 8) | p[i]; return x; }
This still wouldn't handle the case of mixed LE and BE accesses though?
ATB,
Mark.
On Mon, Mar 20, 2017 at 09:04:49AM +0000, Mark Cave-Ayland wrote:
- uint32_t buf, count;
- fw_cfg_read(FW_CFG_FILE_DIR, (char *)&buf, sizeof(uint32_t));
- count = __be32_to_cpu(buf);
It's a lot cleaner to use e.g.
count = fw_cfg_read32(FW_CFG_FILE_DIR);
Yeah I know what you mean here. The reason for the above was that the existing registers are LE, except for the fw_cfg API which is BE. And since this was the only BE I just added it straight in...
I'm not sure I'm too excited about splitting this into a separate function
You can make it inline if you are worried about performance.
although I do see that I'm missing a couple of __be*_to_cpu() wrappers in this patch. I'll do a resend later.
Yeah, you cannot make such mistakes if you do all accesses with explicit endianness ;-)
and ideally you wouldn't use byte-swapping things at all:
uint32_t fw_cfg_read32(unsigned char *p) { uint32_t x = 0; for (int i = 0; i < 4; i++_ x = (x << 8) | p[i]; return x; }
This still wouldn't handle the case of mixed LE and BE accesses though?
Are there both LE and BE things in fw_cfg? read_be32 etc. then?
Segher
On 20/03/17 09:21, Segher Boessenkool wrote:
On Mon, Mar 20, 2017 at 09:04:49AM +0000, Mark Cave-Ayland wrote:
- uint32_t buf, count;
- fw_cfg_read(FW_CFG_FILE_DIR, (char *)&buf, sizeof(uint32_t));
- count = __be32_to_cpu(buf);
It's a lot cleaner to use e.g.
count = fw_cfg_read32(FW_CFG_FILE_DIR);
Yeah I know what you mean here. The reason for the above was that the existing registers are LE, except for the fw_cfg API which is BE. And since this was the only BE I just added it straight in...
I'm not sure I'm too excited about splitting this into a separate function
You can make it inline if you are worried about performance.
I'm not really worried about the performance, just really trying to minimise the code churn for something that only has one caller. And if someone else does need it at a later date, it's fairly easy to refactor out.
although I do see that I'm missing a couple of __be*_to_cpu() wrappers in this patch. I'll do a resend later.
Yeah, you cannot make such mistakes if you do all accesses with explicit endianness ;-)
Indeed, although coincidentally both SPARC and PPC are BE which is why I didn't notice this in my local tests :)
and ideally you wouldn't use byte-swapping things at all:
uint32_t fw_cfg_read32(unsigned char *p) { uint32_t x = 0; for (int i = 0; i < 4; i++_ x = (x << 8) | p[i]; return x; }
This still wouldn't handle the case of mixed LE and BE accesses though?
Are there both LE and BE things in fw_cfg? read_be32 etc. then?
Oh yes - this is where it gets really fun. The original fw_cfg interface (cmd < 0x20) is LE, while the fw_cfg file API uses BE. And to make this even more fun, SPARC fw_cfg accesses are done via ioport accesses which are LE, while PPC fw_cfg accesses are done via MMIO which is BE.
ATB,
Mark.
Implement fw-cfg-read-file Forth word as a simple wrapper around fw_cfg_read_file().
Signed-off-by: Mark Cave-Ayland mark.cave-ayland@ilande.co.uk --- drivers/fw_cfg.c | 23 +++++++++++++++++++++++ include/arch/common/fw_cfg.h | 1 + libopenbios/init.c | 7 ++++++- 3 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/drivers/fw_cfg.c b/drivers/fw_cfg.c index f930ea8..d72461d 100644 --- a/drivers/fw_cfg.c +++ b/drivers/fw_cfg.c @@ -114,6 +114,29 @@ fw_cfg_read_file(const char *filename, uint32_t *size) return NULL; }
+// +// ( fname fnamelen -- buf buflen -1 | 0 ) +// + +void +forth_fw_cfg_read_file(void) +{ + char *filename = pop_fstr_copy(); + char *buffer; + uint32_t size; + + buffer = fw_cfg_read_file(filename, &size); + if (buffer) { + PUSH(pointer2cell(buffer)); + PUSH(size); + PUSH(-1); + + return; + } + + PUSH(0); +} + void fw_cfg_init(void) { diff --git a/include/arch/common/fw_cfg.h b/include/arch/common/fw_cfg.h index 1c43a9d..cd2183a 100644 --- a/include/arch/common/fw_cfg.h +++ b/include/arch/common/fw_cfg.h @@ -100,6 +100,7 @@ typedef struct FWCfgFiles {
unsigned int fw_cfg_find_file(const char *filename, uint16_t *select, uint32_t *size); char *fw_cfg_read_file(const char *filename, uint32_t *size); +void forth_fw_cfg_read_file(void); #endif /* NO_OPENBIOS_PROTOS */
#endif diff --git a/libopenbios/init.c b/libopenbios/init.c index 8882bf3..aa99608 100644 --- a/libopenbios/init.c +++ b/libopenbios/init.c @@ -18,6 +18,8 @@ #include "libopenbios/openbios.h" #include "libopenbios/bindings.h" #include "libopenbios/initprogram.h" +#define NO_QEMU_PROTOS +#include "arch/common/fw_cfg.h"
void openbios_init( void ) @@ -25,7 +27,10 @@ openbios_init( void ) // Bind the saved program state context into Forth PUSH(pointer2cell((void *)&__context)); feval("['] __context cell+ !"); - + + // Bind the Forth fw_cfg file interface + bind_func("fw-cfg-read-file", forth_fw_cfg_read_file); + // Bind the C implementation of (init-program) into Forth bind_func("(init-program)", init_program);
-
Mark Cave-Ayland -
Segher Boessenkool