* [Qemu-devel] [PATCH v2 1/1] riscv: Ensure the kernel start address is correctly cast
@ 2019-01-12 1:17 Alistair Francis
2019-01-14 10:58 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 5+ messages in thread
From: Alistair Francis @ 2019-01-12 1:17 UTC (permalink / raw)
To: qemu-devel@nongnu.org, qemu-riscv@nongnu.org
Cc: Alistair Francis, alistair23@gmail.com, agraf@suse.de
Cast the kernel start address to the target bit length.
This ensures that we calculate the initrd offset to a valid address for
the architecture.
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Suggested-by: Alexander Graf <agraf@suse.de>
Reported-by: Alexander Graf <agraf@suse.de>
---
v2:
- Remove old comment
hw/riscv/sifive_e.c | 2 +-
hw/riscv/sifive_u.c | 2 +-
hw/riscv/spike.c | 2 +-
hw/riscv/virt.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
index 5d9d65ff29..e5d7fc548e 100644
--- a/hw/riscv/sifive_e.c
+++ b/hw/riscv/sifive_e.c
@@ -74,7 +74,7 @@ static const struct MemmapEntry {
[SIFIVE_E_DTIM] = { 0x80000000, 0x4000 }
};
-static uint64_t load_kernel(const char *kernel_filename)
+static target_ulong load_kernel(const char *kernel_filename)
{
uint64_t kernel_entry, kernel_high;
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 3bd3b67507..3b3acec377 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -65,7 +65,7 @@ static const struct MemmapEntry {
#define GEM_REVISION 0x10070109
-static uint64_t load_kernel(const char *kernel_filename)
+static target_ulong load_kernel(const char *kernel_filename)
{
uint64_t kernel_entry, kernel_high;
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 268df04c3c..79cb4c1282 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -53,7 +53,7 @@ static const struct MemmapEntry {
[SPIKE_DRAM] = { 0x80000000, 0x0 },
};
-static uint64_t load_kernel(const char *kernel_filename)
+static target_ulong load_kernel(const char *kernel_filename)
{
uint64_t kernel_entry, kernel_high;
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index e7f0716fb6..648462b18c 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -62,7 +62,7 @@ static const struct MemmapEntry {
[VIRT_PCIE_ECAM] = { 0x30000000, 0x10000000 },
};
-static uint64_t load_kernel(const char *kernel_filename)
+static target_ulong load_kernel(const char *kernel_filename)
{
uint64_t kernel_entry, kernel_high;
--
2.19.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH v2 1/1] riscv: Ensure the kernel start address is correctly cast
2019-01-12 1:17 [Qemu-devel] [PATCH v2 1/1] riscv: Ensure the kernel start address is correctly cast Alistair Francis
@ 2019-01-14 10:58 ` Philippe Mathieu-Daudé
2019-01-15 21:09 ` Alistair Francis
0 siblings, 1 reply; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-01-14 10:58 UTC (permalink / raw)
To: Alistair Francis, qemu-devel@nongnu.org, qemu-riscv@nongnu.org
Cc: alistair23@gmail.com, agraf@suse.de
Hi Alistair,
On 1/12/19 2:17 AM, Alistair Francis wrote:
> Cast the kernel start address to the target bit length.
>
> This ensures that we calculate the initrd offset to a valid address for
> the architecture.
Can you add an example of the failure symptoms?
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> Suggested-by: Alexander Graf <agraf@suse.de>
> Reported-by: Alexander Graf <agraf@suse.de>
> ---
> v2:
> - Remove old comment
> hw/riscv/sifive_e.c | 2 +-
> hw/riscv/sifive_u.c | 2 +-
> hw/riscv/spike.c | 2 +-
> hw/riscv/virt.c | 2 +-
> 4 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
> index 5d9d65ff29..e5d7fc548e 100644
> --- a/hw/riscv/sifive_e.c
> +++ b/hw/riscv/sifive_e.c
> @@ -74,7 +74,7 @@ static const struct MemmapEntry {
> [SIFIVE_E_DTIM] = { 0x80000000, 0x4000 }
> };
>
> -static uint64_t load_kernel(const char *kernel_filename)
> +static target_ulong load_kernel(const char *kernel_filename)
> {
> uint64_t kernel_entry, kernel_high;
Shouldn't you update load_elf() and co now to take target_ulong
arguments? This would fix this error generically for all archs.
>
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index 3bd3b67507..3b3acec377 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -65,7 +65,7 @@ static const struct MemmapEntry {
>
> #define GEM_REVISION 0x10070109
>
> -static uint64_t load_kernel(const char *kernel_filename)
> +static target_ulong load_kernel(const char *kernel_filename)
> {
> uint64_t kernel_entry, kernel_high;
>
> diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
> index 268df04c3c..79cb4c1282 100644
> --- a/hw/riscv/spike.c
> +++ b/hw/riscv/spike.c
> @@ -53,7 +53,7 @@ static const struct MemmapEntry {
> [SPIKE_DRAM] = { 0x80000000, 0x0 },
> };
>
> -static uint64_t load_kernel(const char *kernel_filename)
> +static target_ulong load_kernel(const char *kernel_filename)
> {
> uint64_t kernel_entry, kernel_high;
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index e7f0716fb6..648462b18c 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -62,7 +62,7 @@ static const struct MemmapEntry {
> [VIRT_PCIE_ECAM] = { 0x30000000, 0x10000000 },
> };
>
> -static uint64_t load_kernel(const char *kernel_filename)
> +static target_ulong load_kernel(const char *kernel_filename)
> {
> uint64_t kernel_entry, kernel_high;
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH v2 1/1] riscv: Ensure the kernel start address is correctly cast
2019-01-14 10:58 ` Philippe Mathieu-Daudé
@ 2019-01-15 21:09 ` Alistair Francis
2019-01-24 2:00 ` Palmer Dabbelt
0 siblings, 1 reply; 5+ messages in thread
From: Alistair Francis @ 2019-01-15 21:09 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Alistair Francis, qemu-devel@nongnu.org, qemu-riscv@nongnu.org,
agraf@suse.de
On Mon, Jan 14, 2019 at 2:58 AM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> Hi Alistair,
>
> On 1/12/19 2:17 AM, Alistair Francis wrote:
> > Cast the kernel start address to the target bit length.
> >
> > This ensures that we calculate the initrd offset to a valid address for
> > the architecture.
>
> Can you add an example of the failure symptoms?
I can.
>
> >
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > Suggested-by: Alexander Graf <agraf@suse.de>
> > Reported-by: Alexander Graf <agraf@suse.de>
> > ---
> > v2:
> > - Remove old comment
> > hw/riscv/sifive_e.c | 2 +-
> > hw/riscv/sifive_u.c | 2 +-
> > hw/riscv/spike.c | 2 +-
> > hw/riscv/virt.c | 2 +-
> > 4 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
> > index 5d9d65ff29..e5d7fc548e 100644
> > --- a/hw/riscv/sifive_e.c
> > +++ b/hw/riscv/sifive_e.c
> > @@ -74,7 +74,7 @@ static const struct MemmapEntry {
> > [SIFIVE_E_DTIM] = { 0x80000000, 0x4000 }
> > };
> >
> > -static uint64_t load_kernel(const char *kernel_filename)
> > +static target_ulong load_kernel(const char *kernel_filename)
> > {
> > uint64_t kernel_entry, kernel_high;
>
> Shouldn't you update load_elf() and co now to take target_ulong
> arguments? This would fix this error generically for all archs.
That is an option, but as load_elf() is called by every other machine
I don't want to break them. It's entirely possible that other machines
rely on this behaviour and changing it will break them.
Alistair
>
> >
> > diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> > index 3bd3b67507..3b3acec377 100644
> > --- a/hw/riscv/sifive_u.c
> > +++ b/hw/riscv/sifive_u.c
> > @@ -65,7 +65,7 @@ static const struct MemmapEntry {
> >
> > #define GEM_REVISION 0x10070109
> >
> > -static uint64_t load_kernel(const char *kernel_filename)
> > +static target_ulong load_kernel(const char *kernel_filename)
> > {
> > uint64_t kernel_entry, kernel_high;
> >
> > diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
> > index 268df04c3c..79cb4c1282 100644
> > --- a/hw/riscv/spike.c
> > +++ b/hw/riscv/spike.c
> > @@ -53,7 +53,7 @@ static const struct MemmapEntry {
> > [SPIKE_DRAM] = { 0x80000000, 0x0 },
> > };
> >
> > -static uint64_t load_kernel(const char *kernel_filename)
> > +static target_ulong load_kernel(const char *kernel_filename)
> > {
> > uint64_t kernel_entry, kernel_high;
> >
> > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> > index e7f0716fb6..648462b18c 100644
> > --- a/hw/riscv/virt.c
> > +++ b/hw/riscv/virt.c
> > @@ -62,7 +62,7 @@ static const struct MemmapEntry {
> > [VIRT_PCIE_ECAM] = { 0x30000000, 0x10000000 },
> > };
> >
> > -static uint64_t load_kernel(const char *kernel_filename)
> > +static target_ulong load_kernel(const char *kernel_filename)
> > {
> > uint64_t kernel_entry, kernel_high;
> >
> >
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH v2 1/1] riscv: Ensure the kernel start address is correctly cast
2019-01-15 21:09 ` Alistair Francis
@ 2019-01-24 2:00 ` Palmer Dabbelt
2019-01-24 17:36 ` Alistair Francis
0 siblings, 1 reply; 5+ messages in thread
From: Palmer Dabbelt @ 2019-01-24 2:00 UTC (permalink / raw)
To: alistair23; +Cc: philmd, qemu-riscv, Alistair Francis, qemu-devel, agraf
On Tue, 15 Jan 2019 13:09:28 PST (-0800), alistair23@gmail.com wrote:
> On Mon, Jan 14, 2019 at 2:58 AM Philippe Mathieu-Daudé
> <philmd@redhat.com> wrote:
>>
>> Hi Alistair,
>>
>> On 1/12/19 2:17 AM, Alistair Francis wrote:
>> > Cast the kernel start address to the target bit length.
>> >
>> > This ensures that we calculate the initrd offset to a valid address for
>> > the architecture.
>>
>> Can you add an example of the failure symptoms?
>
> I can.
Should I be waiting for a v3?
>
>>
>> >
>> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
>> > Suggested-by: Alexander Graf <agraf@suse.de>
>> > Reported-by: Alexander Graf <agraf@suse.de>
>> > ---
>> > v2:
>> > - Remove old comment
>> > hw/riscv/sifive_e.c | 2 +-
>> > hw/riscv/sifive_u.c | 2 +-
>> > hw/riscv/spike.c | 2 +-
>> > hw/riscv/virt.c | 2 +-
>> > 4 files changed, 4 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
>> > index 5d9d65ff29..e5d7fc548e 100644
>> > --- a/hw/riscv/sifive_e.c
>> > +++ b/hw/riscv/sifive_e.c
>> > @@ -74,7 +74,7 @@ static const struct MemmapEntry {
>> > [SIFIVE_E_DTIM] = { 0x80000000, 0x4000 }
>> > };
>> >
>> > -static uint64_t load_kernel(const char *kernel_filename)
>> > +static target_ulong load_kernel(const char *kernel_filename)
>> > {
>> > uint64_t kernel_entry, kernel_high;
>>
>> Shouldn't you update load_elf() and co now to take target_ulong
>> arguments? This would fix this error generically for all archs.
>
> That is an option, but as load_elf() is called by every other machine
> I don't want to break them. It's entirely possible that other machines
> rely on this behaviour and changing it will break them.
>
> Alistair
>
>>
>> >
>> > diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
>> > index 3bd3b67507..3b3acec377 100644
>> > --- a/hw/riscv/sifive_u.c
>> > +++ b/hw/riscv/sifive_u.c
>> > @@ -65,7 +65,7 @@ static const struct MemmapEntry {
>> >
>> > #define GEM_REVISION 0x10070109
>> >
>> > -static uint64_t load_kernel(const char *kernel_filename)
>> > +static target_ulong load_kernel(const char *kernel_filename)
>> > {
>> > uint64_t kernel_entry, kernel_high;
>> >
>> > diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
>> > index 268df04c3c..79cb4c1282 100644
>> > --- a/hw/riscv/spike.c
>> > +++ b/hw/riscv/spike.c
>> > @@ -53,7 +53,7 @@ static const struct MemmapEntry {
>> > [SPIKE_DRAM] = { 0x80000000, 0x0 },
>> > };
>> >
>> > -static uint64_t load_kernel(const char *kernel_filename)
>> > +static target_ulong load_kernel(const char *kernel_filename)
>> > {
>> > uint64_t kernel_entry, kernel_high;
>> >
>> > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
>> > index e7f0716fb6..648462b18c 100644
>> > --- a/hw/riscv/virt.c
>> > +++ b/hw/riscv/virt.c
>> > @@ -62,7 +62,7 @@ static const struct MemmapEntry {
>> > [VIRT_PCIE_ECAM] = { 0x30000000, 0x10000000 },
>> > };
>> >
>> > -static uint64_t load_kernel(const char *kernel_filename)
>> > +static target_ulong load_kernel(const char *kernel_filename)
>> > {
>> > uint64_t kernel_entry, kernel_high;
>> >
>> >
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH v2 1/1] riscv: Ensure the kernel start address is correctly cast
2019-01-24 2:00 ` Palmer Dabbelt
@ 2019-01-24 17:36 ` Alistair Francis
0 siblings, 0 replies; 5+ messages in thread
From: Alistair Francis @ 2019-01-24 17:36 UTC (permalink / raw)
To: Palmer Dabbelt
Cc: Philippe Mathieu-Daudé, qemu-riscv, Alistair Francis,
qemu-devel@nongnu.org Developers, Alexander Graf
On Wed, Jan 23, 2019 at 6:00 PM Palmer Dabbelt <palmer@sifive.com> wrote:
>
> On Tue, 15 Jan 2019 13:09:28 PST (-0800), alistair23@gmail.com wrote:
> > On Mon, Jan 14, 2019 at 2:58 AM Philippe Mathieu-Daudé
> > <philmd@redhat.com> wrote:
> >>
> >> Hi Alistair,
> >>
> >> On 1/12/19 2:17 AM, Alistair Francis wrote:
> >> > Cast the kernel start address to the target bit length.
> >> >
> >> > This ensures that we calculate the initrd offset to a valid address for
> >> > the architecture.
> >>
> >> Can you add an example of the failure symptoms?
> >
> > I can.
>
> Should I be waiting for a v3?
You should be, I thought I sent it but I must have forgotten. I'll send it now.
Alistair
>
> >
> >>
> >> >
> >> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> >> > Suggested-by: Alexander Graf <agraf@suse.de>
> >> > Reported-by: Alexander Graf <agraf@suse.de>
> >> > ---
> >> > v2:
> >> > - Remove old comment
> >> > hw/riscv/sifive_e.c | 2 +-
> >> > hw/riscv/sifive_u.c | 2 +-
> >> > hw/riscv/spike.c | 2 +-
> >> > hw/riscv/virt.c | 2 +-
> >> > 4 files changed, 4 insertions(+), 4 deletions(-)
> >> >
> >> > diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
> >> > index 5d9d65ff29..e5d7fc548e 100644
> >> > --- a/hw/riscv/sifive_e.c
> >> > +++ b/hw/riscv/sifive_e.c
> >> > @@ -74,7 +74,7 @@ static const struct MemmapEntry {
> >> > [SIFIVE_E_DTIM] = { 0x80000000, 0x4000 }
> >> > };
> >> >
> >> > -static uint64_t load_kernel(const char *kernel_filename)
> >> > +static target_ulong load_kernel(const char *kernel_filename)
> >> > {
> >> > uint64_t kernel_entry, kernel_high;
> >>
> >> Shouldn't you update load_elf() and co now to take target_ulong
> >> arguments? This would fix this error generically for all archs.
> >
> > That is an option, but as load_elf() is called by every other machine
> > I don't want to break them. It's entirely possible that other machines
> > rely on this behaviour and changing it will break them.
> >
> > Alistair
> >
> >>
> >> >
> >> > diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> >> > index 3bd3b67507..3b3acec377 100644
> >> > --- a/hw/riscv/sifive_u.c
> >> > +++ b/hw/riscv/sifive_u.c
> >> > @@ -65,7 +65,7 @@ static const struct MemmapEntry {
> >> >
> >> > #define GEM_REVISION 0x10070109
> >> >
> >> > -static uint64_t load_kernel(const char *kernel_filename)
> >> > +static target_ulong load_kernel(const char *kernel_filename)
> >> > {
> >> > uint64_t kernel_entry, kernel_high;
> >> >
> >> > diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
> >> > index 268df04c3c..79cb4c1282 100644
> >> > --- a/hw/riscv/spike.c
> >> > +++ b/hw/riscv/spike.c
> >> > @@ -53,7 +53,7 @@ static const struct MemmapEntry {
> >> > [SPIKE_DRAM] = { 0x80000000, 0x0 },
> >> > };
> >> >
> >> > -static uint64_t load_kernel(const char *kernel_filename)
> >> > +static target_ulong load_kernel(const char *kernel_filename)
> >> > {
> >> > uint64_t kernel_entry, kernel_high;
> >> >
> >> > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> >> > index e7f0716fb6..648462b18c 100644
> >> > --- a/hw/riscv/virt.c
> >> > +++ b/hw/riscv/virt.c
> >> > @@ -62,7 +62,7 @@ static const struct MemmapEntry {
> >> > [VIRT_PCIE_ECAM] = { 0x30000000, 0x10000000 },
> >> > };
> >> >
> >> > -static uint64_t load_kernel(const char *kernel_filename)
> >> > +static target_ulong load_kernel(const char *kernel_filename)
> >> > {
> >> > uint64_t kernel_entry, kernel_high;
> >> >
> >> >
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-01-24 17:37 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-12 1:17 [Qemu-devel] [PATCH v2 1/1] riscv: Ensure the kernel start address is correctly cast Alistair Francis
2019-01-14 10:58 ` Philippe Mathieu-Daudé
2019-01-15 21:09 ` Alistair Francis
2019-01-24 2:00 ` Palmer Dabbelt
2019-01-24 17:36 ` Alistair Francis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).