Re: [Qemu-devel] [PATCH] mips: Fix "Unexpected FPU mode"

[Daniel Santos]

Hello,

Sorry for my slow reply.  I don't mind that, as long as I end up being
shown as the author in git. :)  I've never committed from an email
before, so I'm not sure how that works.  Does adding another "From: "
header in the body patch that up with git-am?

I don't know how much I'll be contributing to qemu in the future --
there are a few crazy things I have considered trying to add and I'll be
using qemu quite a bit in the future, so this probably won't be my only
patch.  But in this case, I just *really* needed to get something MIPS
running on my workstation.

Also, as my big "please note", it would be good for somebody more
familiar with that code to make sure anything else that isn't explicitly
initialized will behave correctly after being memset to zero.  When a
zero is OK, I often add an explicit this.that = 0; to clarify the
intention and just expect gcc to compile it away.

Thanks!
Daniel

On 4/23/19 1:00 PM, Aleksandar Markovic wrote:
> On Wed, Apr 17, 2019 at 9:50 PM Daniel Santos  wrote:
>> In load_elf_binary, struct image_info interp_info is used without being
>> properly initialized.  One result is that when the ELF's program header
>> doesn't contain an entry for the ABI flags, then the value of the struct
>> image_info's fp_abi field is set to whatever happened to be in stack
>> memory at the time.
>>
>> This patch both sanitizes interp_info and initializes fp_abi for
>> TARGET_MIPS to MIPS_ABI_FP_UNKNOWN so that when we don't know the FP
>> ABI, we don't just blow up.  Currently, this bug is a complete stopper
>> for some MIPS binaries.
>>
>> ***PLEASE NOTE***
>> There may be other bugs as a result of struct image_info interp_info
>> fields not being properly initialized -- this patch only addresses the
>> fp_abi field.  I reccomend somebody who knows the code better than I
>> audit this function and the whole of that execution path.
>>
>> Fixes bug #1825002 and affects 3.1.0 and 4.x, reccomend backporting to
>> 3.1.0.
>>
>> Signed-off-by: Daniel Santos 
>> ---
> Daniel, not knowing that you already send this patch, I included it in another
> series (with different title and commit message, but the same content):
>
> https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg03813.html
>
> Please let's track this patch there.
>
> I will change the commit message to bring it closer to the yours version
> in the next version of the series, and I will review the patch from MIPS
> point of view.
>
> Just advice for the future: Before sending patches to qemu-devel,
> check what are maintainers for the applicable code. There is even a
> script for that: /scripts/get_maintainers.pl
>
> There are also other rules and conventiones, and all of them are
> mentioned on the page "How to submit a patch" on QEMU web site.
>
> But, in any case, many thanks for discovering and reporting the bug,
> and even devising the fix!
>
> Yours,
> Aleksandar
>
>>  linux-user/elfload.c | 5 +
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
>> index c1a26021f8..7f09d572a2 100644
>> --- a/linux-user/elfload.c
>> +++ b/linux-user/elfload.c
>> @@ -2698,6 +2698,11 @@ int load_elf_binary(struct linux_binprm *bprm, struct 
>> image_info *info)
>>  char *elf_interpreter = NULL;
>>  char *scratch;
>>
>> +memset(&interp_info, 0, sizeof(interp_info));
>> +#ifdef TARGET_MIPS
>> +interp_info.fp_abi = MIPS_ABI_FP_UNKNOWN;
>> +#endif
>> +
>>  info->start_mmap = (abi_ulong)ELF_START_MMAP;
>>
>>  load_elf_image(bprm->filename, bprm->fd, info,
>> --
>> 2.19.2
>>
>>

Re: [Qemu-devel] [PATCH] mips: Fix "Unexpected FPU mode"

[Aleksandar Markovic]

On Wed, Apr 17, 2019 at 9:50 PM Daniel Santos  wrote:
>
> In load_elf_binary, struct image_info interp_info is used without being
> properly initialized.  One result is that when the ELF's program header
> doesn't contain an entry for the ABI flags, then the value of the struct
> image_info's fp_abi field is set to whatever happened to be in stack
> memory at the time.
>
> This patch both sanitizes interp_info and initializes fp_abi for
> TARGET_MIPS to MIPS_ABI_FP_UNKNOWN so that when we don't know the FP
> ABI, we don't just blow up.  Currently, this bug is a complete stopper
> for some MIPS binaries.
>
> ***PLEASE NOTE***
> There may be other bugs as a result of struct image_info interp_info
> fields not being properly initialized -- this patch only addresses the
> fp_abi field.  I reccomend somebody who knows the code better than I
> audit this function and the whole of that execution path.
>
> Fixes bug #1825002 and affects 3.1.0 and 4.x, reccomend backporting to
> 3.1.0.
>
> Signed-off-by: Daniel Santos 
> ---

Daniel, not knowing that you already send this patch, I included it in another
series (with different title and commit message, but the same content):

https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg03813.html

Please let's track this patch there.

I will change the commit message to bring it closer to the yours version
in the next version of the series, and I will review the patch from MIPS
point of view.

Just advice for the future: Before sending patches to qemu-devel,
check what are maintainers for the applicable code. There is even a
script for that: /scripts/get_maintainers.pl

There are also other rules and conventiones, and all of them are
mentioned on the page "How to submit a patch" on QEMU web site.

But, in any case, many thanks for discovering and reporting the bug,
and even devising the fix!

Yours,
Aleksandar

>  linux-user/elfload.c | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index c1a26021f8..7f09d572a2 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -2698,6 +2698,11 @@ int load_elf_binary(struct linux_binprm *bprm, struct 
> image_info *info)
>  char *elf_interpreter = NULL;
>  char *scratch;
>
> +memset(&interp_info, 0, sizeof(interp_info));
> +#ifdef TARGET_MIPS
> +interp_info.fp_abi = MIPS_ABI_FP_UNKNOWN;
> +#endif
> +
>  info->start_mmap = (abi_ulong)ELF_START_MMAP;
>
>  load_elf_image(bprm->filename, bprm->fd, info,
> --
> 2.19.2
>
>

Re: [Qemu-devel] [PATCH] mips: Decide to map PAGE_EXEC in map_address

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/20190423092602.1254515-1-jakub.jer...@kernkonzept.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20190423092602.1254515-1-jakub.jer...@kernkonzept.com
Subject: [Qemu-devel] [PATCH] mips: Decide to map PAGE_EXEC in map_address

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]
patchew/1555669092-10351-1-git-send-email-frederic.kon...@adacore.com -> 
patchew/1555669092-10351-1-git-send-email-frederic.kon...@adacore.com
 t [tag update]
patchew/1556018982-3715-1-git-send-email-aleksandar.marko...@rt-rk.com -> 
patchew/1556018982-3715-1-git-send-email-aleksandar.marko...@rt-rk.com
 t [tag update]
patchew/20190422145838.70903-1-vsement...@virtuozzo.com -> 
patchew/20190422145838.70903-1-vsement...@virtuozzo.com
 * [new tag]   
patchew/20190423092602.1254515-1-jakub.jer...@kernkonzept.com -> 
patchew/20190423092602.1254515-1-jakub.jer...@kernkonzept.com
 t [tag update]patchew/20190423102145.14812-1-f4...@amsat.org -> 
patchew/20190423102145.14812-1-f4...@amsat.org
 * [new tag]   
patchew/20190423132004.13725-1-marcandre.lur...@redhat.com -> 
patchew/20190423132004.13725-1-marcandre.lur...@redhat.com
Switched to a new branch 'test'
5b60f02658 mips: Decide to map PAGE_EXEC in map_address

=== OUTPUT BEGIN ===
ERROR: braces {} are necessary for all arms of this statement
#47: FILE: target/mips/helper.c:104:
+if (!(n ? tlb->XI1 : tlb->XI0))
[...]

total: 1 errors, 0 warnings, 52 lines checked

Commit 5b60f02658bb (mips: Decide to map PAGE_EXEC in map_address) has style 
problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190423092602.1254515-1-jakub.jer...@kernkonzept.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

[Qemu-devel] [PATCH] mips: Decide to map PAGE_EXEC in map_address

[Jakub Jermář]

This commit addresses QEMU Bug #1825311:

  mips_cpu_handle_mmu_fault renders all accessed pages executable

It allows finer-grained control over whether the accessed page should be
executable by moving the decision to the underlying map_address
function, which has more information for this.

As a result, pages that have the XI bit set in the TLB and are accessed
for read/write, don't suddenly end up being executable.

Signed-off-by: Jakub Jermář 
---
 target/mips/helper.c | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/target/mips/helper.c b/target/mips/helper.c
index c44cdca3b5..f182935fcf 100644
--- a/target/mips/helper.c
+++ b/target/mips/helper.c
@@ -43,7 +43,7 @@ int no_mmu_map_address (CPUMIPSState *env, hwaddr *physical, 
int *prot,
 target_ulong address, int rw, int access_type)
 {
 *physical = address;
-*prot = PAGE_READ | PAGE_WRITE;
+*prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
 return TLBRET_MATCH;
 }
 
@@ -61,7 +61,7 @@ int fixed_mmu_map_address (CPUMIPSState *env, hwaddr 
*physical, int *prot,
 else
 *physical = address;
 
-*prot = PAGE_READ | PAGE_WRITE;
+*prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
 return TLBRET_MATCH;
 }
 
@@ -101,6 +101,8 @@ int r4k_map_address (CPUMIPSState *env, hwaddr *physical, 
int *prot,
 *prot = PAGE_READ;
 if (n ? tlb->D1 : tlb->D0)
 *prot |= PAGE_WRITE;
+if (!(n ? tlb->XI1 : tlb->XI0))
+*prot |= PAGE_EXEC;
 return TLBRET_MATCH;
 }
 return TLBRET_DIRTY;
@@ -182,7 +184,7 @@ static int get_seg_physical_address(CPUMIPSState *env, 
hwaddr *physical,
 } else {
 /* The segment is unmapped */
 *physical = physical_base | (real_address & segmask);
-*prot = PAGE_READ | PAGE_WRITE;
+*prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
 return TLBRET_MATCH;
 }
 }
@@ -913,8 +915,8 @@ int mips_cpu_handle_mmu_fault(CPUState *cs, vaddr address, 
int size, int rw,
 }
 if (ret == TLBRET_MATCH) {
 tlb_set_page(cs, address & TARGET_PAGE_MASK,
- physical & TARGET_PAGE_MASK, prot | PAGE_EXEC,
- mmu_idx, TARGET_PAGE_SIZE);
+ physical & TARGET_PAGE_MASK, prot, mmu_idx,
+ TARGET_PAGE_SIZE);
 ret = 0;
 } else if (ret < 0)
 #endif
@@ -936,8 +938,8 @@ int mips_cpu_handle_mmu_fault(CPUState *cs, vaddr address, 
int size, int rw,
address, rw, access_type, mmu_idx);
 if (ret == TLBRET_MATCH) {
 tlb_set_page(cs, address & TARGET_PAGE_MASK,
-physical & TARGET_PAGE_MASK, prot | PAGE_EXEC,
-mmu_idx, TARGET_PAGE_SIZE);
+physical & TARGET_PAGE_MASK, prot, mmu_idx,
+TARGET_PAGE_SIZE);
 ret = 0;
 return ret;
 }
-- 
2.20.1

Re: [Qemu-devel] [PATCH] mips: Decide to map PAGE_EXEC in map_address

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/20190423103855.1257968-1-jakub.jer...@kernkonzept.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20190423103855.1257968-1-jakub.jer...@kernkonzept.com
Subject: [Qemu-devel] [PATCH] mips: Decide to map PAGE_EXEC in map_address

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]   
patchew/20190423103855.1257968-1-jakub.jer...@kernkonzept.com -> 
patchew/20190423103855.1257968-1-jakub.jer...@kernkonzept.com
Switched to a new branch 'test'
40e0d72157 mips: Decide to map PAGE_EXEC in map_address

=== OUTPUT BEGIN ===
ERROR: braces {} are necessary for all arms of this statement
#47: FILE: target/mips/helper.c:104:
+if (!(n ? tlb->XI1 : tlb->XI0))
[...]

total: 1 errors, 0 warnings, 52 lines checked

Commit 40e0d7215724 (mips: Decide to map PAGE_EXEC in map_address) has style 
problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190423103855.1257968-1-jakub.jer...@kernkonzept.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

[Qemu-devel] [PATCH] mips: Decide to map PAGE_EXEC in map_address

[Jakub Jermář]

This commit addresses QEMU Bug #1825311:

  mips_cpu_handle_mmu_fault renders all accessed pages executable

It allows finer-grained control over whether the accessed page should be
executable by moving the decision to the underlying map_address
function, which has more information for this.

As a result, pages that have the XI bit set in the TLB and are accessed
for read/write, don't suddenly end up being executable.

Signed-off-by: Jakub Jermář 
---
 target/mips/helper.c | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/target/mips/helper.c b/target/mips/helper.c
index c44cdca3b5..f182935fcf 100644
--- a/target/mips/helper.c
+++ b/target/mips/helper.c
@@ -43,7 +43,7 @@ int no_mmu_map_address (CPUMIPSState *env, hwaddr *physical, 
int *prot,
 target_ulong address, int rw, int access_type)
 {
 *physical = address;
-*prot = PAGE_READ | PAGE_WRITE;
+*prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
 return TLBRET_MATCH;
 }
 
@@ -61,7 +61,7 @@ int fixed_mmu_map_address (CPUMIPSState *env, hwaddr 
*physical, int *prot,
 else
 *physical = address;
 
-*prot = PAGE_READ | PAGE_WRITE;
+*prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
 return TLBRET_MATCH;
 }
 
@@ -101,6 +101,8 @@ int r4k_map_address (CPUMIPSState *env, hwaddr *physical, 
int *prot,
 *prot = PAGE_READ;
 if (n ? tlb->D1 : tlb->D0)
 *prot |= PAGE_WRITE;
+if (!(n ? tlb->XI1 : tlb->XI0))
+*prot |= PAGE_EXEC;
 return TLBRET_MATCH;
 }
 return TLBRET_DIRTY;
@@ -182,7 +184,7 @@ static int get_seg_physical_address(CPUMIPSState *env, 
hwaddr *physical,
 } else {
 /* The segment is unmapped */
 *physical = physical_base | (real_address & segmask);
-*prot = PAGE_READ | PAGE_WRITE;
+*prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
 return TLBRET_MATCH;
 }
 }
@@ -913,8 +915,8 @@ int mips_cpu_handle_mmu_fault(CPUState *cs, vaddr address, 
int size, int rw,
 }
 if (ret == TLBRET_MATCH) {
 tlb_set_page(cs, address & TARGET_PAGE_MASK,
- physical & TARGET_PAGE_MASK, prot | PAGE_EXEC,
- mmu_idx, TARGET_PAGE_SIZE);
+ physical & TARGET_PAGE_MASK, prot, mmu_idx,
+ TARGET_PAGE_SIZE);
 ret = 0;
 } else if (ret < 0)
 #endif
@@ -936,8 +938,8 @@ int mips_cpu_handle_mmu_fault(CPUState *cs, vaddr address, 
int size, int rw,
address, rw, access_type, mmu_idx);
 if (ret == TLBRET_MATCH) {
 tlb_set_page(cs, address & TARGET_PAGE_MASK,
-physical & TARGET_PAGE_MASK, prot | PAGE_EXEC,
-mmu_idx, TARGET_PAGE_SIZE);
+physical & TARGET_PAGE_MASK, prot, mmu_idx,
+TARGET_PAGE_SIZE);
 ret = 0;
 return ret;
 }
-- 
2.20.1

[Qemu-devel] [PATCH] mips: Fix "Unexpected FPU mode"

[Daniel Santos]

In load_elf_binary, struct image_info interp_info is used without being
properly initialized.  One result is that when the ELF's program header
doesn't contain an entry for the ABI flags, then the value of the struct
image_info's fp_abi field is set to whatever happened to be in stack
memory at the time.

This patch both sanitizes interp_info and initializes fp_abi for
TARGET_MIPS to MIPS_ABI_FP_UNKNOWN so that when we don't know the FP
ABI, we don't just blow up.  Currently, this bug is a complete stopper
for some MIPS binaries.

***PLEASE NOTE***
There may be other bugs as a result of struct image_info interp_info
fields not being properly initialized -- this patch only addresses the
fp_abi field.  I reccomend somebody who knows the code better than I
audit this function and the whole of that execution path.

Fixes bug #1825002 and affects 3.1.0 and 4.x, reccomend backporting to
3.1.0.

Signed-off-by: Daniel Santos 
---
 linux-user/elfload.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index c1a26021f8..7f09d572a2 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2698,6 +2698,11 @@ int load_elf_binary(struct linux_binprm *bprm, struct 
image_info *info)
 char *elf_interpreter = NULL;
 char *scratch;
 
+memset(&interp_info, 0, sizeof(interp_info));
+#ifdef TARGET_MIPS
+interp_info.fp_abi = MIPS_ABI_FP_UNKNOWN;
+#endif
+
 info->start_mmap = (abi_ulong)ELF_START_MMAP;
 
 load_elf_image(bprm->filename, bprm->fd, info,
-- 
2.19.2

Re: [Qemu-devel] [PATCH] mips-fulong2e: obey -vga none

[Markus Armbruster]

BALATON Zoltan  writes:

> On Tue, 19 Mar 2019, Paolo Bonzini wrote:
>> Do not create an ATI VGA if "-vga none" was passed on the command line.
>>
>> Cc: BALATON Zoltan 
>
> Thanks, I did not know about this variable. Although the real hardware
> has the GPU soldered on the mainboard it makes sense to allow it to be
> disabled in QEMU especially at this stage when Linux kernel has some
> problem with it so this is a good idea.
>
> Reviewed-by: BALATON Zoltan 
>
> When changing it you could also replace the -1 in pci_create with
> PCI_DEVFN(FULONG2E_ATI_SLOT, 0) to match the address the board has or
> should that be a separate patch?

Replacing these -1 in board code is generally a good idea.  I'd make it
a separate patch.

Re: [Qemu-devel] [PATCH] mips-fulong2e: obey -vga none

[BALATON Zoltan]

On Tue, 19 Mar 2019, Paolo Bonzini wrote:

Do not create an ATI VGA if "-vga none" was passed on the command line.

Cc: BALATON Zoltan 


Thanks, I did not know about this variable. Although the real hardware has 
the GPU soldered on the mainboard it makes sense to allow it to be 
disabled in QEMU especially at this stage when Linux kernel has some 
problem with it so this is a good idea.


Reviewed-by: BALATON Zoltan 

When changing it you could also replace the -1 in pci_create with 
PCI_DEVFN(FULONG2E_ATI_SLOT, 0) to match the address the board has or 
should that be a separate patch?


Regards,
BALATON Zoltan


Signed-off-by: Paolo Bonzini 
---
hw/mips/mips_fulong2e.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
index 9d7480ed31..05a5a823a1 100644
--- a/hw/mips/mips_fulong2e.c
+++ b/hw/mips/mips_fulong2e.c
@@ -349,10 +349,12 @@ static void mips_fulong2e_init(MachineState *machine)
   &smbus, &isa_bus);

/* GPU */
-dev = DEVICE(pci_create(pci_bus, -1, "ati-vga"));
-qdev_prop_set_uint32(dev, "vgamem_mb", 16);
-qdev_prop_set_uint16(dev, "x-device-id", 0x5159);
-qdev_init_nofail(dev);
+if (vga_interface_type != VGA_NONE) {
+dev = DEVICE(pci_create(pci_bus, -1, "ati-vga"));
+qdev_prop_set_uint32(dev, "vgamem_mb", 16);
+qdev_prop_set_uint16(dev, "x-device-id", 0x5159);
+qdev_init_nofail(dev);
+}

/* Populate SPD eeprom data */
spd_data = spd_data_generate(DDR, ram_size, &err);

Re: [Qemu-devel] [PATCH] mips-fulong2e: obey -vga none

[Aleksandar Markovic]

> From: Paolo Bonzini 
> Cc: phi...@redhat.com; BALATON Zoltan; Aleksandar Markovic; Aleksandar 
> Rikalo; Aurelien Jarno
> Subject: [PATCH] mips-fulong2e: obey -vga none
> 
> Do not create an ATI VGA if "-vga none" was passed on the command line.
> 
> Cc: BALATON Zoltan 
> Signed-off-by: Paolo Bonzini 
> ---

This looks like a very good point to me.

I am going to wait for comments of others.

Many thanks, Paolo!
Aleksandar

P.S. Balaton Zoltan, what is your first, and what is your last name? I know 
"Zoltan" can be both.

>  hw/mips/mips_fulong2e.c | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
> index 9d7480ed31..05a5a823a1 100644
> --- a/hw/mips/mips_fulong2e.c
> +++ b/hw/mips/mips_fulong2e.c
> @@ -349,10 +349,12 @@ static void mips_fulong2e_init(MachineState *machine)
> &smbus, &isa_bus);
> 
>  /* GPU */
> -dev = DEVICE(pci_create(pci_bus, -1, "ati-vga"));
> -qdev_prop_set_uint32(dev, "vgamem_mb", 16);
> -qdev_prop_set_uint16(dev, "x-device-id", 0x5159);
> -qdev_init_nofail(dev);
> +if (vga_interface_type != VGA_NONE) {
> +dev = DEVICE(pci_create(pci_bus, -1, "ati-vga"));
> +qdev_prop_set_uint32(dev, "vgamem_mb", 16);
> +qdev_prop_set_uint16(dev, "x-device-id", 0x5159);
> +qdev_init_nofail(dev);
> +}
> 
>  /* Populate SPD eeprom data */
>  spd_data = spd_data_generate(DDR, ram_size, &err);
> --
> 2.20.1
> 

[Qemu-devel] [PATCH] mips-fulong2e: obey -vga none

[Paolo Bonzini]

Do not create an ATI VGA if "-vga none" was passed on the command line.

Cc: BALATON Zoltan 
Signed-off-by: Paolo Bonzini 
---
 hw/mips/mips_fulong2e.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
index 9d7480ed31..05a5a823a1 100644
--- a/hw/mips/mips_fulong2e.c
+++ b/hw/mips/mips_fulong2e.c
@@ -349,10 +349,12 @@ static void mips_fulong2e_init(MachineState *machine)
&smbus, &isa_bus);
 
 /* GPU */
-dev = DEVICE(pci_create(pci_bus, -1, "ati-vga"));
-qdev_prop_set_uint32(dev, "vgamem_mb", 16);
-qdev_prop_set_uint16(dev, "x-device-id", 0x5159);
-qdev_init_nofail(dev);
+if (vga_interface_type != VGA_NONE) {
+dev = DEVICE(pci_create(pci_bus, -1, "ati-vga"));
+qdev_prop_set_uint32(dev, "vgamem_mb", 16);
+qdev_prop_set_uint16(dev, "x-device-id", 0x5159);
+qdev_init_nofail(dev);
+}
 
 /* Populate SPD eeprom data */
 spd_data = spd_data_generate(DDR, ram_size, &err);
-- 
2.20.1

Re: [Qemu-devel] [PATCH] mips: implement qmp query-cpu-definitions command

[Pavel Dovgalyuk]

> From: Aleksandar Markovic [mailto:amarko...@wavecomp.com]
> > From: Markus Armbruster 
> > Subject: Re: [Qemu-devel] [PATCH] mips: implement qmp query-cpu-definitions 
> > command
> 
> > Please rebase.  Let me know if you need help.
> 
> Hi, Markus.
> 
> Pavel was probably busy today, so I took the liberty to rebase the patch,
> and here is the v2:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg05158.html

Thank you!

Pavel Dovgalyuk

Re: [Qemu-devel] [PATCH] mips: implement qmp query-cpu-definitions command

[Aleksandar Markovic]

> From: Markus Armbruster 
> Subject: Re: [Qemu-devel] [PATCH] mips: implement qmp query-cpu-definitions 
> command

> Please rebase.  Let me know if you need help.

Hi, Markus.

Pavel was probably busy today, so I took the liberty to rebase the patch,
and here is the v2:

https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg05158.html

I would really appreciate if you, or somebody else knowledgeable in QAPI,
take a look. I did only build tests.

Regards,
Aleksandar

Re: [Qemu-devel] [PATCH] mips: implement qmp query-cpu-definitions command

[Markus Armbruster]

Philippe Mathieu-Daudé  writes:

> Hi Pavel,
>
> On 2/11/19 6:34 AM, Pavel Dovgalyuk wrote:
>> Ping.
>
> You forgot to Cc Aleksandar, to get his MIPS maintainer Ack-by:
>
> ./scripts/get_maintainer.pl -f target/mips/helper.c
> Aleksandar Markovic  (maintainer:MIPS)
>
>> 
>> Pavel Dovgalyuk
>> 
>>> -Original Message-
>>> From: Pavel Dovgalyuk [mailto:pavel.dovga...@ispras.ru]
>>> Sent: Tuesday, February 05, 2019 4:08 PM
>>> To: qemu-devel@nongnu.org
>>> Cc: pavel.dovga...@ispras.ru; arik...@wavecomp.com; 
>>> mdr...@linux.vnet.ibm.com;
>>> arm...@redhat.com; dovga...@ispras.ru; natalia.furs...@ispras.ru; 
>>> ebl...@redhat.com;
>>> aurel...@aurel32.net
>>> Subject: [PATCH] mips: implement qmp query-cpu-definitions command
>>>
>>> This patch enables QMP-based querying of the available CPU types for MIPS
>>> and MIPS64 platforms.
>
> Your patch is a simple copy of the ARM code, so:
>
> Reviewed-by: Philippe Mathieu-Daudé 
>
> Also:
>
> Tested-by: Philippe Mathieu-Daudé 
>
> However this clashes with Marc-André's "qapi: make query-cpu-definitions
> depend on specific targets" posted here by Markus:
> https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg03831.html

This is now in master, merge commit a0430dd8abb.

> I believe your patch will go thru the QMP tree, so you might want to
> rebase on top of the series Markus sent; or see if Markus is OK to do
> the manual cleanup when applying.

Please rebase.  Let me know if you need help.

Re: [Qemu-devel] [PATCH] mips: implement qmp query-cpu-definitions command

[Pavel Dovgalyuk]

> From: Aleksandar Markovic [mailto:amarko...@wavecomp.com]
> > From: Pavel Dovgalyuk [mailto:pavel.dovga...@ispras.ru]
> >
> > This patch enables QMP-based querying of the available CPU types for MIPS
> > and MIPS64 platforms.
> >
> > Signed-off-by: Pavel Dovgalyuk 
> > ---
> >  monitor.c|2 +-
> >  target/mips/helper.c |   33 +
> >  2 files changed, 34 insertions(+), 1 deletion(-)
> >
> 
> Hello, Pavel,
> 
> Thanks for involving in this area!
> 
> I have just a couple of question:
> 
> 1) What are the effects of these two patches on the end user?

This patch make qmp query-cpu-definitions available for the MIPS users.
This command allows requesting possible CPU models with QMP.

> 2) What is the context of these patches? Do you intend to send more related 
> patches in the
> future? Are these patches preconditions for some other not yet implemented 
> features?

Not yet.
We are developing GUI for virtual machine management and debugging
with record-replay feature: https://github.com/ispras/qemu-gui
Therefore we need to request possible CPU and hardware options.

> 3) Why is only target MIPS included? Do other targets need similar 
> improvements?

We use MIPS in our projects. Some other targets need similar improvements, but 
we do
not focus on them right now.

Pavel Dovgalyuk

> 
> Thanks,
> Aleksandar
> 
> 
> > diff --git a/monitor.c b/monitor.c
> > index c09fa63940..25d3b141ad 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -1165,7 +1165,7 @@ static void qmp_unregister_commands_hack(void)
> >  qmp_unregister_command(&qmp_commands, "query-cpu-model-comparison");
> >  #endif
> >  #if !defined(TARGET_PPC) && !defined(TARGET_ARM) && !defined(TARGET_I386) \
> > -&& !defined(TARGET_S390X)
> > +&& !defined(TARGET_S390X) && !defined(TARGET_MIPS)
> >  qmp_unregister_command(&qmp_commands, "query-cpu-definitions");
> >  #endif
> >  }
> > diff --git a/target/mips/helper.c b/target/mips/helper.c
> > index 8988452dbd..c84d056c09 100644
> > --- a/target/mips/helper.c
> > +++ b/target/mips/helper.c
> > @@ -24,6 +24,7 @@
> >  #include "exec/cpu_ldst.h"
> >  #include "exec/log.h"
> >  #include "hw/mips/cpudevs.h"
> > +#include "sysemu/arch_init.h"
> >
> >  enum {
> >  TLBRET_XI = -6,
> > @@ -1472,3 +1473,35 @@ void QEMU_NORETURN 
> > do_raise_exception_err(CPUMIPSState *env,
> >
> >  cpu_loop_exit_restore(cs, pc);
> >  }
> > +
> > +static void mips_cpu_add_definition(gpointer data, gpointer user_data)
> > +{
> > +ObjectClass *oc = data;
> > +CpuDefinitionInfoList **cpu_list = user_data;
> > +CpuDefinitionInfoList *entry;
> > +CpuDefinitionInfo *info;
> > +const char *typename;
> > +
> > +typename = object_class_get_name(oc);
> > +info = g_malloc0(sizeof(*info));
> > +info->name = g_strndup(typename,
> > +   strlen(typename) - strlen("-" TYPE_MIPS_CPU));
> > +info->q_typename = g_strdup(typename);
> > +
> > +entry = g_malloc0(sizeof(*entry));
> > +entry->value = info;
> > +entry->next = *cpu_list;
> > +*cpu_list = entry;
> > +}
> > +
> > +CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
> > +{
> > +CpuDefinitionInfoList *cpu_list = NULL;
> > +GSList *list;
> > +
> > +list = object_class_get_list(TYPE_MIPS_CPU, false);
> > +g_slist_foreach(list, mips_cpu_add_definition, &cpu_list);
> > +g_slist_free(list);
> > +
> > +return cpu_list;
> > +}
> 
> 

Re: [Qemu-devel] [PATCH] mips: implement qmp query-cpu-definitions command

[Aleksandar Markovic]

> From: Pavel Dovgalyuk [mailto:pavel.dovga...@ispras.ru]
> Sent: Tuesday, February 05, 2019 4:08 PM
> To: qemu-devel@nongnu.org
> Cc: pavel.dovga...@ispras.ru; arik...@wavecomp.com; mdr...@linux.vnet.ibm.com;
> arm...@redhat.com; dovga...@ispras.ru; natalia.furs...@ispras.ru; 
> ebl...@redhat.com;
> aurel...@aurel32.net
> Subject: [PATCH] mips: implement qmp query-cpu-definitions command
>
> This patch enables QMP-based querying of the available CPU types for MIPS
> and MIPS64 platforms.
>
> Signed-off-by: Pavel Dovgalyuk 
> ---
>  monitor.c|2 +-
>  target/mips/helper.c |   33 +
>  2 files changed, 34 insertions(+), 1 deletion(-)
>

Hello, Pavel,

Thanks for involving in this area!

I have just a couple of question:

1) What are the effects of these two patches on the end user?

2) What is the context of these patches? Do you intend to send more related 
patches in the future? Are these patches preconditions for some other not yet 
implemented features?

3) Why is only target MIPS included? Do other targets need similar improvements?

Thanks,
Aleksandar


> diff --git a/monitor.c b/monitor.c
> index c09fa63940..25d3b141ad 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -1165,7 +1165,7 @@ static void qmp_unregister_commands_hack(void)
>  qmp_unregister_command(&qmp_commands, "query-cpu-model-comparison");
>  #endif
>  #if !defined(TARGET_PPC) && !defined(TARGET_ARM) && !defined(TARGET_I386) \
> -&& !defined(TARGET_S390X)
> +&& !defined(TARGET_S390X) && !defined(TARGET_MIPS)
>  qmp_unregister_command(&qmp_commands, "query-cpu-definitions");
>  #endif
>  }
> diff --git a/target/mips/helper.c b/target/mips/helper.c
> index 8988452dbd..c84d056c09 100644
> --- a/target/mips/helper.c
> +++ b/target/mips/helper.c
> @@ -24,6 +24,7 @@
>  #include "exec/cpu_ldst.h"
>  #include "exec/log.h"
>  #include "hw/mips/cpudevs.h"
> +#include "sysemu/arch_init.h"
>
>  enum {
>  TLBRET_XI = -6,
> @@ -1472,3 +1473,35 @@ void QEMU_NORETURN do_raise_exception_err(CPUMIPSState 
> *env,
>
>  cpu_loop_exit_restore(cs, pc);
>  }
> +
> +static void mips_cpu_add_definition(gpointer data, gpointer user_data)
> +{
> +ObjectClass *oc = data;
> +CpuDefinitionInfoList **cpu_list = user_data;
> +CpuDefinitionInfoList *entry;
> +CpuDefinitionInfo *info;
> +const char *typename;
> +
> +typename = object_class_get_name(oc);
> +info = g_malloc0(sizeof(*info));
> +info->name = g_strndup(typename,
> +   strlen(typename) - strlen("-" TYPE_MIPS_CPU));
> +info->q_typename = g_strdup(typename);
> +
> +entry = g_malloc0(sizeof(*entry));
> +entry->value = info;
> +entry->next = *cpu_list;
> +*cpu_list = entry;
> +}
> +
> +CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
> +{
> +CpuDefinitionInfoList *cpu_list = NULL;
> +GSList *list;
> +
> +list = object_class_get_list(TYPE_MIPS_CPU, false);
> +g_slist_foreach(list, mips_cpu_add_definition, &cpu_list);
> +g_slist_free(list);
> +
> +return cpu_list;
> +}

Re: [Qemu-devel] [PATCH] mips: implement qmp query-cpu-definitions command

[Philippe Mathieu-Daudé]

Hi Pavel,

On 2/11/19 6:34 AM, Pavel Dovgalyuk wrote:
> Ping.

You forgot to Cc Aleksandar, to get his MIPS maintainer Ack-by:

./scripts/get_maintainer.pl -f target/mips/helper.c
Aleksandar Markovic  (maintainer:MIPS)

> 
> Pavel Dovgalyuk
> 
>> -Original Message-
>> From: Pavel Dovgalyuk [mailto:pavel.dovga...@ispras.ru]
>> Sent: Tuesday, February 05, 2019 4:08 PM
>> To: qemu-devel@nongnu.org
>> Cc: pavel.dovga...@ispras.ru; arik...@wavecomp.com; 
>> mdr...@linux.vnet.ibm.com;
>> arm...@redhat.com; dovga...@ispras.ru; natalia.furs...@ispras.ru; 
>> ebl...@redhat.com;
>> aurel...@aurel32.net
>> Subject: [PATCH] mips: implement qmp query-cpu-definitions command
>>
>> This patch enables QMP-based querying of the available CPU types for MIPS
>> and MIPS64 platforms.

Your patch is a simple copy of the ARM code, so:

Reviewed-by: Philippe Mathieu-Daudé 

Also:

Tested-by: Philippe Mathieu-Daudé 

However this clashes with Marc-André's "qapi: make query-cpu-definitions
depend on specific targets" posted here by Markus:
https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg03831.html

I believe your patch will go thru the QMP tree, so you might want to
rebase on top of the series Markus sent; or see if Markus is OK to do
the manual cleanup when applying.

Regards,

Phil.

>>
>> Signed-off-by: Pavel Dovgalyuk 
>> ---
>>  monitor.c|2 +-
>>  target/mips/helper.c |   33 +
>>  2 files changed, 34 insertions(+), 1 deletion(-)
>>
>> diff --git a/monitor.c b/monitor.c
>> index c09fa63940..25d3b141ad 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -1165,7 +1165,7 @@ static void qmp_unregister_commands_hack(void)
>>  qmp_unregister_command(&qmp_commands, "query-cpu-model-comparison");
>>  #endif
>>  #if !defined(TARGET_PPC) && !defined(TARGET_ARM) && !defined(TARGET_I386) \
>> -&& !defined(TARGET_S390X)
>> +&& !defined(TARGET_S390X) && !defined(TARGET_MIPS)
>>  qmp_unregister_command(&qmp_commands, "query-cpu-definitions");
>>  #endif
>>  }
>> diff --git a/target/mips/helper.c b/target/mips/helper.c
>> index 8988452dbd..c84d056c09 100644
>> --- a/target/mips/helper.c
>> +++ b/target/mips/helper.c
>> @@ -24,6 +24,7 @@
>>  #include "exec/cpu_ldst.h"
>>  #include "exec/log.h"
>>  #include "hw/mips/cpudevs.h"
>> +#include "sysemu/arch_init.h"
>>
>>  enum {
>>  TLBRET_XI = -6,
>> @@ -1472,3 +1473,35 @@ void QEMU_NORETURN 
>> do_raise_exception_err(CPUMIPSState *env,
>>
>>  cpu_loop_exit_restore(cs, pc);
>>  }
>> +
>> +static void mips_cpu_add_definition(gpointer data, gpointer user_data)
>> +{
>> +ObjectClass *oc = data;
>> +CpuDefinitionInfoList **cpu_list = user_data;
>> +CpuDefinitionInfoList *entry;
>> +CpuDefinitionInfo *info;
>> +const char *typename;
>> +
>> +typename = object_class_get_name(oc);
>> +info = g_malloc0(sizeof(*info));
>> +info->name = g_strndup(typename,
>> +   strlen(typename) - strlen("-" TYPE_MIPS_CPU));
>> +info->q_typename = g_strdup(typename);
>> +
>> +entry = g_malloc0(sizeof(*entry));
>> +entry->value = info;
>> +entry->next = *cpu_list;
>> +*cpu_list = entry;
>> +}
>> +
>> +CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
>> +{
>> +CpuDefinitionInfoList *cpu_list = NULL;
>> +GSList *list;
>> +
>> +list = object_class_get_list(TYPE_MIPS_CPU, false);
>> +g_slist_foreach(list, mips_cpu_add_definition, &cpu_list);
>> +g_slist_free(list);
>> +
>> +return cpu_list;
>> +}
> 
> 
> 

Re: [Qemu-devel] [PATCH] mips: implement qmp query-cpu-definitions command

[Pavel Dovgalyuk]

Ping.


Pavel Dovgalyuk

> -Original Message-
> From: Pavel Dovgalyuk [mailto:pavel.dovga...@ispras.ru]
> Sent: Tuesday, February 05, 2019 4:08 PM
> To: qemu-devel@nongnu.org
> Cc: pavel.dovga...@ispras.ru; arik...@wavecomp.com; mdr...@linux.vnet.ibm.com;
> arm...@redhat.com; dovga...@ispras.ru; natalia.furs...@ispras.ru; 
> ebl...@redhat.com;
> aurel...@aurel32.net
> Subject: [PATCH] mips: implement qmp query-cpu-definitions command
> 
> This patch enables QMP-based querying of the available CPU types for MIPS
> and MIPS64 platforms.
> 
> Signed-off-by: Pavel Dovgalyuk 
> ---
>  monitor.c|2 +-
>  target/mips/helper.c |   33 +
>  2 files changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/monitor.c b/monitor.c
> index c09fa63940..25d3b141ad 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -1165,7 +1165,7 @@ static void qmp_unregister_commands_hack(void)
>  qmp_unregister_command(&qmp_commands, "query-cpu-model-comparison");
>  #endif
>  #if !defined(TARGET_PPC) && !defined(TARGET_ARM) && !defined(TARGET_I386) \
> -&& !defined(TARGET_S390X)
> +&& !defined(TARGET_S390X) && !defined(TARGET_MIPS)
>  qmp_unregister_command(&qmp_commands, "query-cpu-definitions");
>  #endif
>  }
> diff --git a/target/mips/helper.c b/target/mips/helper.c
> index 8988452dbd..c84d056c09 100644
> --- a/target/mips/helper.c
> +++ b/target/mips/helper.c
> @@ -24,6 +24,7 @@
>  #include "exec/cpu_ldst.h"
>  #include "exec/log.h"
>  #include "hw/mips/cpudevs.h"
> +#include "sysemu/arch_init.h"
> 
>  enum {
>  TLBRET_XI = -6,
> @@ -1472,3 +1473,35 @@ void QEMU_NORETURN do_raise_exception_err(CPUMIPSState 
> *env,
> 
>  cpu_loop_exit_restore(cs, pc);
>  }
> +
> +static void mips_cpu_add_definition(gpointer data, gpointer user_data)
> +{
> +ObjectClass *oc = data;
> +CpuDefinitionInfoList **cpu_list = user_data;
> +CpuDefinitionInfoList *entry;
> +CpuDefinitionInfo *info;
> +const char *typename;
> +
> +typename = object_class_get_name(oc);
> +info = g_malloc0(sizeof(*info));
> +info->name = g_strndup(typename,
> +   strlen(typename) - strlen("-" TYPE_MIPS_CPU));
> +info->q_typename = g_strdup(typename);
> +
> +entry = g_malloc0(sizeof(*entry));
> +entry->value = info;
> +entry->next = *cpu_list;
> +*cpu_list = entry;
> +}
> +
> +CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
> +{
> +CpuDefinitionInfoList *cpu_list = NULL;
> +GSList *list;
> +
> +list = object_class_get_list(TYPE_MIPS_CPU, false);
> +g_slist_foreach(list, mips_cpu_add_definition, &cpu_list);
> +g_slist_free(list);
> +
> +return cpu_list;
> +}

[Qemu-devel] [PATCH] mips: implement qmp query-cpu-definitions command

[Pavel Dovgalyuk]

This patch enables QMP-based querying of the available CPU types for MIPS
and MIPS64 platforms.

Signed-off-by: Pavel Dovgalyuk 
---
 monitor.c|2 +-
 target/mips/helper.c |   33 +
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/monitor.c b/monitor.c
index c09fa63940..25d3b141ad 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1165,7 +1165,7 @@ static void qmp_unregister_commands_hack(void)
 qmp_unregister_command(&qmp_commands, "query-cpu-model-comparison");
 #endif
 #if !defined(TARGET_PPC) && !defined(TARGET_ARM) && !defined(TARGET_I386) \
-&& !defined(TARGET_S390X)
+&& !defined(TARGET_S390X) && !defined(TARGET_MIPS)
 qmp_unregister_command(&qmp_commands, "query-cpu-definitions");
 #endif
 }
diff --git a/target/mips/helper.c b/target/mips/helper.c
index 8988452dbd..c84d056c09 100644
--- a/target/mips/helper.c
+++ b/target/mips/helper.c
@@ -24,6 +24,7 @@
 #include "exec/cpu_ldst.h"
 #include "exec/log.h"
 #include "hw/mips/cpudevs.h"
+#include "sysemu/arch_init.h"
 
 enum {
 TLBRET_XI = -6,
@@ -1472,3 +1473,35 @@ void QEMU_NORETURN do_raise_exception_err(CPUMIPSState 
*env,
 
 cpu_loop_exit_restore(cs, pc);
 }
+
+static void mips_cpu_add_definition(gpointer data, gpointer user_data)
+{
+ObjectClass *oc = data;
+CpuDefinitionInfoList **cpu_list = user_data;
+CpuDefinitionInfoList *entry;
+CpuDefinitionInfo *info;
+const char *typename;
+
+typename = object_class_get_name(oc);
+info = g_malloc0(sizeof(*info));
+info->name = g_strndup(typename,
+   strlen(typename) - strlen("-" TYPE_MIPS_CPU));
+info->q_typename = g_strdup(typename);
+
+entry = g_malloc0(sizeof(*entry));
+entry->value = info;
+entry->next = *cpu_list;
+*cpu_list = entry;
+}
+
+CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
+{
+CpuDefinitionInfoList *cpu_list = NULL;
+GSList *list;
+
+list = object_class_get_list(TYPE_MIPS_CPU, false);
+g_slist_foreach(list, mips_cpu_add_definition, &cpu_list);
+g_slist_free(list);
+
+return cpu_list;
+}

Re: [Qemu-devel] [PATCH] mips: Improve macro parenthesization

[Yongbok Kim]

On 19/09/2017 15:13, Eric Blake wrote:
> Although none of the existing macro call-sites were broken,
> it's always better to write macros that properly parenthesize
> arguments that can be complex expressions, so that the intended
> order of operations is not broken.
> 
> Signed-off-by: Eric Blake 
> ---
>  target/mips/dsp_helper.c | 56 
> 
>  1 file changed, 28 insertions(+), 28 deletions(-)
> 
> diff --git a/target/mips/dsp_helper.c b/target/mips/dsp_helper.c
> index dc707934ea..f152fea34a 100644
> --- a/target/mips/dsp_helper.c
> +++ b/target/mips/dsp_helper.c
> @@ -45,9 +45,9 @@ typedef union {
>  } DSP64Value;
> 
>  /*** MIPS DSP internal functions begin ***/
> -#define MIPSDSP_ABS(x) (((x) >= 0) ? x : -x)
> -#define MIPSDSP_OVERFLOW_ADD(a, b, c, d) (~(a ^ b) & (a ^ c) & d)
> -#define MIPSDSP_OVERFLOW_SUB(a, b, c, d) ((a ^ b) & (a ^ c) & d)
> +#define MIPSDSP_ABS(x) (((x) >= 0) ? (x) : -(x))
> +#define MIPSDSP_OVERFLOW_ADD(a, b, c, d) (~((a) ^ (b)) & ((a) ^ (c)) & (d))
> +#define MIPSDSP_OVERFLOW_SUB(a, b, c, d) (((a) ^ (b)) & ((a) ^ (c)) & (d))
> 
>  static inline void set_DSPControl_overflow_flag(uint32_t flag, int position,
>  CPUMIPSState *env)
> @@ -1047,47 +1047,47 @@ static inline int32_t mipsdsp_cmpu_lt(uint32_t a, 
> uint32_t b)
> 
>  #define MIPSDSP_SPLIT32_8(num, a, b, c, d)  \
>  do {\
> -a = (num >> 24) & MIPSDSP_Q0;   \
> -b = (num >> 16) & MIPSDSP_Q0;   \
> -c = (num >> 8) & MIPSDSP_Q0;\
> -d = num & MIPSDSP_Q0;   \
> +a = ((num) >> 24) & MIPSDSP_Q0; \
> +b = ((num) >> 16) & MIPSDSP_Q0; \
> +c = ((num) >> 8) & MIPSDSP_Q0;  \
> +d = (num) & MIPSDSP_Q0; \
>  } while (0)
> 
>  #define MIPSDSP_SPLIT32_16(num, a, b)   \
>  do {\
> -a = (num >> 16) & MIPSDSP_LO;   \
> -b = num & MIPSDSP_LO;   \
> +a = ((num) >> 16) & MIPSDSP_LO; \
> +b = (num) & MIPSDSP_LO; \
>  } while (0)
> 
> -#define MIPSDSP_RETURN32_8(a, b, c, d)  ((target_long)(int32_t) \
> - (((uint32_t)a << 24) | \
> - (((uint32_t)b << 16) | \
> - (((uint32_t)c << 8) |  \
> -  ((uint32_t)d & 0xFF)
> -#define MIPSDSP_RETURN32_16(a, b)   ((target_long)(int32_t) \
> - (((uint32_t)a << 16) | \
> -  ((uint32_t)b & 0x)))
> +#define MIPSDSP_RETURN32_8(a, b, c, d)  ((target_long)(int32_t) \
> + (((uint32_t)(a) << 24) |   \
> +  ((uint32_t)(b) << 16) |   \
> +  ((uint32_t)(c) << 8) |\
> +  ((uint32_t)(d) & 0xFF)))
> +#define MIPSDSP_RETURN32_16(a, b)   ((target_long)(int32_t) \
> + (((uint32_t)(a) << 16) |   \
> +  ((uint32_t)(b) & 0x)))
> 
>  #ifdef TARGET_MIPS64
>  #define MIPSDSP_SPLIT64_16(num, a, b, c, d)  \
>  do { \
> -a = (num >> 48) & MIPSDSP_LO;\
> -b = (num >> 32) & MIPSDSP_LO;\
> -c = (num >> 16) & MIPSDSP_LO;\
> -d = num & MIPSDSP_LO;\
> +a = ((num) >> 48) & MIPSDSP_LO;  \
> +b = ((num) >> 32) & MIPSDSP_LO;  \
> +c = ((num) >> 16) & MIPSDSP_LO;  \
> +d = (num) & MIPSDSP_LO;  \
>  } while (0)
> 
>  #define MIPSDSP_SPLIT64_32(num, a, b)   \
>  do {\
> -a = (num >> 32) & MIPSDSP_LLO;  \
> -b = num & MIPSDSP_LLO;  \
> +a = ((num) >> 32) & MIPSDSP_LLO;\
> +b = (num) & MIPSDSP_LLO;\
>  } while (0)
> 
> -#define MIPSDSP_RETURN64_16(a, b, c, d) (((uint64_t)a << 48) | \
> - ((uint64_t)b << 32) | \
> - ((uint64_t)c << 16) | \
> - (uint64_t)d)
> -#define MIPSDSP_RETURN64_32(a, b)   (((uint64_t)a << 32) | (uint64_t)b)
> +#define MIPSDSP_RETURN64_16(a, b, c, d) (((uint64_t)(a) << 48) |\
> + ((uint64_t)(b) << 32) |\
> + ((uint64_t)(c) << 16) |\
> + (uint64_t)(d))
> +#define MIPSDSP_RETURN64_32(a, b)   (((uint64_t)(a) << 32) | 
> (uint64_t)(b))
>  #endif
> 
>  /** DSP Arithmetic Sub-class insns **/
> 

Reviewed-

[Qemu-devel] [PATCH] mips: Improve macro parenthesization

[Eric Blake]

Although none of the existing macro call-sites were broken,
it's always better to write macros that properly parenthesize
arguments that can be complex expressions, so that the intended
order of operations is not broken.

Signed-off-by: Eric Blake 
---
 target/mips/dsp_helper.c | 56 
 1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/target/mips/dsp_helper.c b/target/mips/dsp_helper.c
index dc707934ea..f152fea34a 100644
--- a/target/mips/dsp_helper.c
+++ b/target/mips/dsp_helper.c
@@ -45,9 +45,9 @@ typedef union {
 } DSP64Value;

 /*** MIPS DSP internal functions begin ***/
-#define MIPSDSP_ABS(x) (((x) >= 0) ? x : -x)
-#define MIPSDSP_OVERFLOW_ADD(a, b, c, d) (~(a ^ b) & (a ^ c) & d)
-#define MIPSDSP_OVERFLOW_SUB(a, b, c, d) ((a ^ b) & (a ^ c) & d)
+#define MIPSDSP_ABS(x) (((x) >= 0) ? (x) : -(x))
+#define MIPSDSP_OVERFLOW_ADD(a, b, c, d) (~((a) ^ (b)) & ((a) ^ (c)) & (d))
+#define MIPSDSP_OVERFLOW_SUB(a, b, c, d) (((a) ^ (b)) & ((a) ^ (c)) & (d))

 static inline void set_DSPControl_overflow_flag(uint32_t flag, int position,
 CPUMIPSState *env)
@@ -1047,47 +1047,47 @@ static inline int32_t mipsdsp_cmpu_lt(uint32_t a, 
uint32_t b)

 #define MIPSDSP_SPLIT32_8(num, a, b, c, d)  \
 do {\
-a = (num >> 24) & MIPSDSP_Q0;   \
-b = (num >> 16) & MIPSDSP_Q0;   \
-c = (num >> 8) & MIPSDSP_Q0;\
-d = num & MIPSDSP_Q0;   \
+a = ((num) >> 24) & MIPSDSP_Q0; \
+b = ((num) >> 16) & MIPSDSP_Q0; \
+c = ((num) >> 8) & MIPSDSP_Q0;  \
+d = (num) & MIPSDSP_Q0; \
 } while (0)

 #define MIPSDSP_SPLIT32_16(num, a, b)   \
 do {\
-a = (num >> 16) & MIPSDSP_LO;   \
-b = num & MIPSDSP_LO;   \
+a = ((num) >> 16) & MIPSDSP_LO; \
+b = (num) & MIPSDSP_LO; \
 } while (0)

-#define MIPSDSP_RETURN32_8(a, b, c, d)  ((target_long)(int32_t) \
- (((uint32_t)a << 24) | \
- (((uint32_t)b << 16) | \
- (((uint32_t)c << 8) |  \
-  ((uint32_t)d & 0xFF)
-#define MIPSDSP_RETURN32_16(a, b)   ((target_long)(int32_t) \
- (((uint32_t)a << 16) | \
-  ((uint32_t)b & 0x)))
+#define MIPSDSP_RETURN32_8(a, b, c, d)  ((target_long)(int32_t) \
+ (((uint32_t)(a) << 24) |   \
+  ((uint32_t)(b) << 16) |   \
+  ((uint32_t)(c) << 8) |\
+  ((uint32_t)(d) & 0xFF)))
+#define MIPSDSP_RETURN32_16(a, b)   ((target_long)(int32_t) \
+ (((uint32_t)(a) << 16) |   \
+  ((uint32_t)(b) & 0x)))

 #ifdef TARGET_MIPS64
 #define MIPSDSP_SPLIT64_16(num, a, b, c, d)  \
 do { \
-a = (num >> 48) & MIPSDSP_LO;\
-b = (num >> 32) & MIPSDSP_LO;\
-c = (num >> 16) & MIPSDSP_LO;\
-d = num & MIPSDSP_LO;\
+a = ((num) >> 48) & MIPSDSP_LO;  \
+b = ((num) >> 32) & MIPSDSP_LO;  \
+c = ((num) >> 16) & MIPSDSP_LO;  \
+d = (num) & MIPSDSP_LO;  \
 } while (0)

 #define MIPSDSP_SPLIT64_32(num, a, b)   \
 do {\
-a = (num >> 32) & MIPSDSP_LLO;  \
-b = num & MIPSDSP_LLO;  \
+a = ((num) >> 32) & MIPSDSP_LLO;\
+b = (num) & MIPSDSP_LLO;\
 } while (0)

-#define MIPSDSP_RETURN64_16(a, b, c, d) (((uint64_t)a << 48) | \
- ((uint64_t)b << 32) | \
- ((uint64_t)c << 16) | \
- (uint64_t)d)
-#define MIPSDSP_RETURN64_32(a, b)   (((uint64_t)a << 32) | (uint64_t)b)
+#define MIPSDSP_RETURN64_16(a, b, c, d) (((uint64_t)(a) << 48) |\
+ ((uint64_t)(b) << 32) |\
+ ((uint64_t)(c) << 16) |\
+ (uint64_t)(d))
+#define MIPSDSP_RETURN64_32(a, b)   (((uint64_t)(a) << 32) | (uint64_t)(b))
 #endif

 /** DSP Arithmetic Sub-class insns **/
-- 
2.13.5

[Qemu-devel] [PATCH] mips/malta: load the initrd at the end of the low memory

[Aurelien Jarno]

Currently the malta board is loading the initrd just after the kernel.
This doesn't work for kaslr enabled kernels, as the initrd ends-up being
overwritten.

Move the initrd at the end of the low memory, that should leave a
sufficient gap for kaslr.

Signed-off-by: Aurelien Jarno 
---
 hw/mips/mips_malta.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 95cdabb2dd..dad2f37fb1 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -841,8 +841,9 @@ static int64_t load_kernel (void)
 if (loaderparams.initrd_filename) {
 initrd_size = get_image_size (loaderparams.initrd_filename);
 if (initrd_size > 0) {
-initrd_offset = (kernel_high + ~INITRD_PAGE_MASK) & 
INITRD_PAGE_MASK;
-if (initrd_offset + initrd_size > ram_size) {
+initrd_offset = (loaderparams.ram_low_size - initrd_size
+ - ~INITRD_PAGE_MASK) & INITRD_PAGE_MASK;
+if (kernel_high >= initrd_offset) {
 fprintf(stderr,
 "qemu: memory too small for initial ram disk '%s'\n",
 loaderparams.initrd_filename);
-- 
2.11.0

Re: [Qemu-devel] [PATCH] mips: set CP0 Debug DExcCode for SDBBP instruction

[Aurelien Jarno]

On 2017-05-02 15:03, Pavel Dovgalyuk wrote:
> From: Pavel Dovgalyuk 
> 
> This patch fixes setting DExcCode field of CP0 Debug register
> when SDBBP instruction is executed. According to EJTAG specification,
> this field must be set to the value 9 (Bp).
> 
> Signed-off-by: Pavel Dovgalyuk 
> ---
>  target/mips/helper.c |2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/target/mips/helper.c b/target/mips/helper.c
> index e359ca3b44..166f0d1243 100644
> --- a/target/mips/helper.c
> +++ b/target/mips/helper.c
> @@ -627,6 +627,8 @@ void mips_cpu_do_interrupt(CPUState *cs)
>  goto set_DEPC;
>  case EXCP_DBp:
>  env->CP0_Debug |= 1 << CP0DB_DBp;
> +/* Setup DExcCode - SDBBP instruction */
> +env->CP0_Debug = (env->CP0_Debug & ~(0x1fULL << CP0DB_DEC)) | 9 << 
> CP0DB_DEC;
>  goto set_DEPC;
>  case EXCP_DDBS:
>  env->CP0_Debug |= 1 << CP0DB_DDBS;
> 

Reviewed-by: Aurelien Jarno 

-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net

Re: [Qemu-devel] [PATCH] mips: set CP0 Debug DExcCode for SDBBP instruction

[Philippe Mathieu-Daudé]

On 05/02/2017 09:03 AM, Pavel Dovgalyuk wrote:

From: Pavel Dovgalyuk 

This patch fixes setting DExcCode field of CP0 Debug register
when SDBBP instruction is executed. According to EJTAG specification,
this field must be set to the value 9 (Bp).

Signed-off-by: Pavel Dovgalyuk 


Reviewed-by: Philippe Mathieu-Daudé 


---
 target/mips/helper.c |2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/mips/helper.c b/target/mips/helper.c
index e359ca3b44..166f0d1243 100644
--- a/target/mips/helper.c
+++ b/target/mips/helper.c
@@ -627,6 +627,8 @@ void mips_cpu_do_interrupt(CPUState *cs)
 goto set_DEPC;
 case EXCP_DBp:
 env->CP0_Debug |= 1 << CP0DB_DBp;
+/* Setup DExcCode - SDBBP instruction */
+env->CP0_Debug = (env->CP0_Debug & ~(0x1fULL << CP0DB_DEC)) | 9 << 
CP0DB_DEC;
 goto set_DEPC;
 case EXCP_DDBS:
 env->CP0_Debug |= 1 << CP0DB_DDBS;

[Qemu-devel] [PATCH] mips: set CP0 Debug DExcCode for SDBBP instruction

[Pavel Dovgalyuk]

From: Pavel Dovgalyuk 

This patch fixes setting DExcCode field of CP0 Debug register
when SDBBP instruction is executed. According to EJTAG specification,
this field must be set to the value 9 (Bp).

Signed-off-by: Pavel Dovgalyuk 
---
 target/mips/helper.c |2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/mips/helper.c b/target/mips/helper.c
index e359ca3b44..166f0d1243 100644
--- a/target/mips/helper.c
+++ b/target/mips/helper.c
@@ -627,6 +627,8 @@ void mips_cpu_do_interrupt(CPUState *cs)
 goto set_DEPC;
 case EXCP_DBp:
 env->CP0_Debug |= 1 << CP0DB_DBp;
+/* Setup DExcCode - SDBBP instruction */
+env->CP0_Debug = (env->CP0_Debug & ~(0x1fULL << CP0DB_DEC)) | 9 << 
CP0DB_DEC;
 goto set_DEPC;
 case EXCP_DDBS:
 env->CP0_Debug |= 1 << CP0DB_DDBS;

Re: [Qemu-devel] [PATCH] mips: Clean up includes

[Leon Alrae]

On 18/01/16 17:35, Peter Maydell wrote:
> Clean up includes so that osdep.h is included first and headers
> which it implies are not included manually.
> 
> This commit was created with scripts/clean-includes.
> 
> Signed-off-by: Peter Maydell 
> ---
>  disas/mips.c | 1 +
>  hw/mips/addr.c   | 1 +
>  hw/mips/cputimer.c   | 1 +
>  hw/mips/gt64xxx_pci.c| 1 +
>  hw/mips/mips_fulong2e.c  | 1 +
>  hw/mips/mips_int.c   | 1 +
>  hw/mips/mips_jazz.c  | 1 +
>  hw/mips/mips_malta.c | 1 +
>  hw/mips/mips_mipssim.c   | 1 +
>  hw/mips/mips_r4k.c   | 1 +
>  target-mips/cpu.c| 1 +
>  target-mips/dsp_helper.c | 1 +
>  target-mips/gdbstub.c| 2 +-
>  target-mips/helper.c | 6 +-
>  target-mips/kvm.c| 2 +-
>  target-mips/lmi_helper.c | 1 +
>  target-mips/machine.c| 1 +
>  target-mips/mips-semi.c  | 2 +-
>  target-mips/msa_helper.c | 1 +
>  target-mips/op_helper.c  | 2 +-
>  target-mips/translate.c  | 1 +
>  21 files changed, 21 insertions(+), 9 deletions(-)

Applied to target-mips tree, thanks.

Regards,
Leon

[Qemu-devel] [PATCH] mips: Clean up includes

[Peter Maydell]

Clean up includes so that osdep.h is included first and headers
which it implies are not included manually.

This commit was created with scripts/clean-includes.

Signed-off-by: Peter Maydell 
---
 disas/mips.c | 1 +
 hw/mips/addr.c   | 1 +
 hw/mips/cputimer.c   | 1 +
 hw/mips/gt64xxx_pci.c| 1 +
 hw/mips/mips_fulong2e.c  | 1 +
 hw/mips/mips_int.c   | 1 +
 hw/mips/mips_jazz.c  | 1 +
 hw/mips/mips_malta.c | 1 +
 hw/mips/mips_mipssim.c   | 1 +
 hw/mips/mips_r4k.c   | 1 +
 target-mips/cpu.c| 1 +
 target-mips/dsp_helper.c | 1 +
 target-mips/gdbstub.c| 2 +-
 target-mips/helper.c | 6 +-
 target-mips/kvm.c| 2 +-
 target-mips/lmi_helper.c | 1 +
 target-mips/machine.c| 1 +
 target-mips/mips-semi.c  | 2 +-
 target-mips/msa_helper.c | 1 +
 target-mips/op_helper.c  | 2 +-
 target-mips/translate.c  | 1 +
 21 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/disas/mips.c b/disas/mips.c
index bf0bbaf..0e488d8 100644
--- a/disas/mips.c
+++ b/disas/mips.c
@@ -19,6 +19,7 @@ GNU General Public License for more details.
 You should have received a copy of the GNU General Public License
 along with this program; if not, see .  */
 
+#include "qemu/osdep.h"
 #include "disas/bfd.h"
 
 /* mips.h.  Mips opcode list for GDB, the GNU debugger.
diff --git a/hw/mips/addr.c b/hw/mips/addr.c
index ff3b952..e4e86b4 100644
--- a/hw/mips/addr.c
+++ b/hw/mips/addr.c
@@ -20,6 +20,7 @@
  * THE SOFTWARE.
  */
 
+#include "qemu/osdep.h"
 #include "hw/hw.h"
 #include "hw/mips/cpudevs.h"
 
diff --git a/hw/mips/cputimer.c b/hw/mips/cputimer.c
index f046588..efb227d 100644
--- a/hw/mips/cputimer.c
+++ b/hw/mips/cputimer.c
@@ -20,6 +20,7 @@
  * THE SOFTWARE.
  */
 
+#include "qemu/osdep.h"
 #include "hw/hw.h"
 #include "hw/mips/cpudevs.h"
 #include "qemu/timer.h"
diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index c1f3c9c..3f4523d 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -22,6 +22,7 @@
  * THE SOFTWARE.
  */
 
+#include "qemu/osdep.h"
 #include "hw/hw.h"
 #include "hw/mips/mips.h"
 #include "hw/pci/pci.h"
diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
index 5988a88..6748d89 100644
--- a/hw/mips/mips_fulong2e.c
+++ b/hw/mips/mips_fulong2e.c
@@ -18,6 +18,7 @@
  * http://www.loongsondeveloper.com/doc/Loongson2EUserGuide.pdf
  */
 
+#include "qemu/osdep.h"
 #include "hw/hw.h"
 #include "hw/i386/pc.h"
 #include "hw/char/serial.h"
diff --git a/hw/mips/mips_int.c b/hw/mips/mips_int.c
index d740046..59081f9 100644
--- a/hw/mips/mips_int.c
+++ b/hw/mips/mips_int.c
@@ -20,6 +20,7 @@
  * THE SOFTWARE.
  */
 
+#include "qemu/osdep.h"
 #include "hw/hw.h"
 #include "hw/mips/cpudevs.h"
 #include "cpu.h"
diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
index 1cfbaa6..62527fd 100644
--- a/hw/mips/mips_jazz.c
+++ b/hw/mips/mips_jazz.c
@@ -22,6 +22,7 @@
  * THE SOFTWARE.
  */
 
+#include "qemu/osdep.h"
 #include "hw/hw.h"
 #include "hw/mips/mips.h"
 #include "hw/mips/cpudevs.h"
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 91c36ba..c5da83f 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -22,6 +22,7 @@
  * THE SOFTWARE.
  */
 
+#include "qemu/osdep.h"
 #include "hw/hw.h"
 #include "hw/i386/pc.h"
 #include "hw/char/serial.h"
diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c
index 23b35be..8951ae9 100644
--- a/hw/mips/mips_mipssim.c
+++ b/hw/mips/mips_mipssim.c
@@ -24,6 +24,7 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
+#include "qemu/osdep.h"
 #include "hw/hw.h"
 #include "hw/mips/mips.h"
 #include "hw/mips/cpudevs.h"
diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
index 2d4e038..b6625ae 100644
--- a/hw/mips/mips_r4k.c
+++ b/hw/mips/mips_r4k.c
@@ -7,6 +7,7 @@
  * All peripherial devices are attached to this "bus" with
  * the standard PC ISA addresses.
 */
+#include "qemu/osdep.h"
 #include "hw/hw.h"
 #include "hw/mips/mips.h"
 #include "hw/mips/cpudevs.h"
diff --git a/target-mips/cpu.c b/target-mips/cpu.c
index 639a24b..0b3f130 100644
--- a/target-mips/cpu.c
+++ b/target-mips/cpu.c
@@ -18,6 +18,7 @@
  * 
  */
 
+#include "qemu/osdep.h"
 #include "cpu.h"
 #include "kvm_mips.h"
 #include "qemu-common.h"
diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c
index 46528de..df7d220 100644
--- a/target-mips/dsp_helper.c
+++ b/target-mips/dsp_helper.c
@@ -17,6 +17,7 @@
  * License along with this library; if not, see .
  */
 
+#include "qemu/osdep.h"
 #include "cpu.h"
 #include "exec/helper-proto.h"
 #include "qemu/bitops.h"
diff --git a/target-mips/gdbstub.c b/target-mips/gdbstub.c
index 9845d88..b0b4a32 100644
--- a/target-mips/gdbstub.c
+++ b/target-mips/gdbstub.c
@@ -17,7 +17,7 @@
  * You should have received a copy of the GNU Lesser General Public
  * License along with this library; if not, see 

Re: [Qemu-devel] [PATCH] MIPS: exceptions handling in icount mode

[Aurelien Jarno]

On 2015-06-15 10:48, Pavel Dovgaluk wrote:
> > From: Aurelien Jarno [mailto:aurel...@aurel32.net]
> > On 2015-06-15 07:53, Pavel Dovgaluk wrote:
> > > > From: Aurelien Jarno [mailto:aurel...@aurel32.net]
> > > > On 2015-06-10 11:33, Pavel Dovgalyuk wrote:
> > > > > This patch fixes exception handling in MIPS.
> > > > > MIPS instructions generate several types of exceptions.
> > > > > When exception is generated, it breaks the execution of the current 
> > > > > translation
> > > > > block. Implementation of the exceptions handling in MIPS does not 
> > > > > correctly
> > > > > restore icount for the instruction which caused the exception. In 
> > > > > most cases
> > > > > icount will be decreased by the value equal to the size of TB.
> > > >
> > > > I don't think it is correct. There is no real point of always doing
> > > > retranslation for an exception triggered from the helpers, especially
> > > > when the CPU state has been saved before anyway?
> > >
> > > As you know, icount is processed as follows:
> > >
> > > TB:
> > > if icount < n then exit
> > > icount -= n
> > > instr1
> > > instr2
> > > ...
> > > instrn
> > > exit
> > >
> > > When one of the instructions initiates an exception, then icount should 
> > > be restored
> > > and adjusted number of instructions should be subtracted instead of 
> > > initial n.
> > >
> > > E.g., tlb_fill function passes retaddr to raise_exception, which allows 
> > > restoring
> > > current instructions in TB and correct icount calculation.
> > >
> > > When exception triggered with other function (e.g. by embedding call to
> > > helper_raise_exception_err into TB), then PC is not passed as retaddr and
> > > correct icount is not recovered.
> > >
> > > This behavior leads to incorrect values of timers and non-deterministic 
> > > execution
> > > of the code.
> > 
> > Ok, this therefore doesn't looks something MIPS specific, but rather a
> > flaw in the icount design. Instead of fixing blindly one target, we
> > should try to fix it globally, or if not possible at least agree on a
> > way to fix that for all target and provide the infrastructure for that
> > (for example provide load/store functions which accept a return
> > address). Paolo any opinion on that?
> 
> Recovering from is a tricky mechanism. It can break the correct execution
> if used inaccurately even when icount is disabled.
> I already posted a patch for maskmov instruction in i386:
> http://lists.nongnu.org/archive/html/qemu-devel/2014-09/msg02960.html

One solution there is to save the CPU state before calling the maskmov
helper. That would fix the bug, but not the icount problem you reported.

If we decided that we must always be able to do retranslation, we should
go with the second possibility from Richard:

| (2) Add helpers that accept the GETRA value from the top-level helper.  And 
not
| hidden within a macro or always_inline function.  This helps us see what
| portions of the code have been audited for the new interface.  This will
| involve quite a bit more code churn, but shouldn't been too difficult for any
| single function.

And we should do it a way that it works for both user and softmmu modes,
to avoid too many #ifdef in the targets code, which in general is a
source of bugs.

-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net

Re: [Qemu-devel] [PATCH] MIPS: exceptions handling in icount mode

[Aurelien Jarno]

On 2015-06-15 10:39, Pavel Dovgaluk wrote:
> > From: Aurelien Jarno [mailto:aurel...@aurel32.net]
> > On 2015-06-15 07:53, Pavel Dovgaluk wrote:
> > > > From: Aurelien Jarno [mailto:aurel...@aurel32.net]
> > > > On 2015-06-10 11:33, Pavel Dovgalyuk wrote:
> > > > > This patch fixes exception handling in MIPS.
> > > > > MIPS instructions generate several types of exceptions.
> > > > > When exception is generated, it breaks the execution of the current 
> > > > > translation
> > > > > block. Implementation of the exceptions handling in MIPS does not 
> > > > > correctly
> > > > > restore icount for the instruction which caused the exception. In 
> > > > > most cases
> > > > > icount will be decreased by the value equal to the size of TB.
> > > >
> > > > I don't think it is correct. There is no real point of always doing
> > > > retranslation for an exception triggered from the helpers, especially
> > > > when the CPU state has been saved before anyway?
> > >
> > > As you know, icount is processed as follows:
> > >
> > > TB:
> > > if icount < n then exit
> > > icount -= n
> > > instr1
> > > instr2
> > > ...
> > > instrn
> > > exit
> > >
> > > When one of the instructions initiates an exception, then icount should 
> > > be restored
> > > and adjusted number of instructions should be subtracted instead of 
> > > initial n.
> > >
> > > E.g., tlb_fill function passes retaddr to raise_exception, which allows 
> > > restoring
> > > current instructions in TB and correct icount calculation.
> > >
> > > When exception triggered with other function (e.g. by embedding call to
> > > helper_raise_exception_err into TB), then PC is not passed as retaddr and
> > > correct icount is not recovered.
> > >
> > > This behavior leads to incorrect values of timers and non-deterministic 
> > > execution
> > > of the code.
> > 
> > Ok, this therefore doesn't looks something MIPS specific, but rather a
> > flaw in the icount design. Instead of fixing blindly one target, we
> > should try to fix it globally, or if not possible at least agree on a
> > way to fix that for all target and provide the infrastructure for that
> > (for example provide load/store functions which accept a return
> > address). Paolo any opinion on that?
> > 
> > Also retranslation has a cost (actually on MIPS we spend more time in
> > *retranslation* than in translation due to the way the MMU works), we
> > should avoid it if we already have to save the CPU state for another
> > reason. At least your patch should remove the code saving the CPU state
> > when possible if the helper uses retranslation instead.
> 
> Ok, I'll remove CPU saving where it is not actually used because of the 
> exception.
> 
> > > > > This patch passes pointer to the translation block internals to the 
> > > > > exception
> > > > > handler. It allows correct restoring of the icount value.
> > > >
> > > > Your patch doesn't do that for all the helpers, for example all the
> > > > memory access helpers. It probably improves the situation but therefore
> > > > doesn't fix it.
> > >
> > > Right, I missed these helpers. I'll add them in the next version.
> > 
> > Except we currently don't provide a way in the softmmu code to provide a
> > return address...
> 
> Softmmu passes return address into tlb_fill function, which passes it to 
> raise_exception.
> I also fixed HELPER_LD_ATOMIC and HELPER_ST_ATOMIC to recover PC value.
> Do you mean something different?

Yes, for example in your patch:

> diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
> index 73a8e45..1b798a2 100644
> --- a/target-mips/op_helper.c
> +++ b/target-mips/op_helper.c
> @@ -309,7 +281,7 @@ target_ulong helper_##name(CPUMIPSState *env, 
> target_ulong arg, int mem_idx)  \
>  {
>  \
>  if (arg & almask) {  
>  \
>  env->CP0_BadVAddr = arg; 
>  \
> -helper_raise_exception(env, EXCP_AdEL);  
>  \
> +do_raise_exception(env, EXCP_AdEL, GETPC()); 
>  \
>  }
>  \
>  env->lladdr = do_translate_address(env, arg, 0); 
>  \
>  env->llval = do_##insn(env, arg, mem_idx);   
>  \

do_##insn is actually a load/store instruction, but you doesn't pass the
return address as an argument (and you can't with the current code).
 
-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net

Re: [Qemu-devel] [PATCH] MIPS: exceptions handling in icount mode

[Pavel Dovgaluk]

> From: Aurelien Jarno [mailto:aurel...@aurel32.net]
> On 2015-06-15 07:53, Pavel Dovgaluk wrote:
> > > From: Aurelien Jarno [mailto:aurel...@aurel32.net]
> > > On 2015-06-10 11:33, Pavel Dovgalyuk wrote:
> > > > This patch fixes exception handling in MIPS.
> > > > MIPS instructions generate several types of exceptions.
> > > > When exception is generated, it breaks the execution of the current 
> > > > translation
> > > > block. Implementation of the exceptions handling in MIPS does not 
> > > > correctly
> > > > restore icount for the instruction which caused the exception. In most 
> > > > cases
> > > > icount will be decreased by the value equal to the size of TB.
> > >
> > > I don't think it is correct. There is no real point of always doing
> > > retranslation for an exception triggered from the helpers, especially
> > > when the CPU state has been saved before anyway?
> >
> > As you know, icount is processed as follows:
> >
> > TB:
> > if icount < n then exit
> > icount -= n
> > instr1
> > instr2
> > ...
> > instrn
> > exit
> >
> > When one of the instructions initiates an exception, then icount should be 
> > restored
> > and adjusted number of instructions should be subtracted instead of initial 
> > n.
> >
> > E.g., tlb_fill function passes retaddr to raise_exception, which allows 
> > restoring
> > current instructions in TB and correct icount calculation.
> >
> > When exception triggered with other function (e.g. by embedding call to
> > helper_raise_exception_err into TB), then PC is not passed as retaddr and
> > correct icount is not recovered.
> >
> > This behavior leads to incorrect values of timers and non-deterministic 
> > execution
> > of the code.
> 
> Ok, this therefore doesn't looks something MIPS specific, but rather a
> flaw in the icount design. Instead of fixing blindly one target, we
> should try to fix it globally, or if not possible at least agree on a
> way to fix that for all target and provide the infrastructure for that
> (for example provide load/store functions which accept a return
> address). Paolo any opinion on that?

Recovering from is a tricky mechanism. It can break the correct execution
if used inaccurately even when icount is disabled.
I already posted a patch for maskmov instruction in i386:
http://lists.nongnu.org/archive/html/qemu-devel/2014-09/msg02960.html

Pavel Dovgalyuk

Re: [Qemu-devel] [PATCH] MIPS: exceptions handling in icount mode

[Pavel Dovgaluk]

> From: Aurelien Jarno [mailto:aurel...@aurel32.net]
> On 2015-06-15 07:53, Pavel Dovgaluk wrote:
> > > From: Aurelien Jarno [mailto:aurel...@aurel32.net]
> > > On 2015-06-10 11:33, Pavel Dovgalyuk wrote:
> > > > This patch fixes exception handling in MIPS.
> > > > MIPS instructions generate several types of exceptions.
> > > > When exception is generated, it breaks the execution of the current 
> > > > translation
> > > > block. Implementation of the exceptions handling in MIPS does not 
> > > > correctly
> > > > restore icount for the instruction which caused the exception. In most 
> > > > cases
> > > > icount will be decreased by the value equal to the size of TB.
> > >
> > > I don't think it is correct. There is no real point of always doing
> > > retranslation for an exception triggered from the helpers, especially
> > > when the CPU state has been saved before anyway?
> >
> > As you know, icount is processed as follows:
> >
> > TB:
> > if icount < n then exit
> > icount -= n
> > instr1
> > instr2
> > ...
> > instrn
> > exit
> >
> > When one of the instructions initiates an exception, then icount should be 
> > restored
> > and adjusted number of instructions should be subtracted instead of initial 
> > n.
> >
> > E.g., tlb_fill function passes retaddr to raise_exception, which allows 
> > restoring
> > current instructions in TB and correct icount calculation.
> >
> > When exception triggered with other function (e.g. by embedding call to
> > helper_raise_exception_err into TB), then PC is not passed as retaddr and
> > correct icount is not recovered.
> >
> > This behavior leads to incorrect values of timers and non-deterministic 
> > execution
> > of the code.
> 
> Ok, this therefore doesn't looks something MIPS specific, but rather a
> flaw in the icount design. Instead of fixing blindly one target, we
> should try to fix it globally, or if not possible at least agree on a
> way to fix that for all target and provide the infrastructure for that
> (for example provide load/store functions which accept a return
> address). Paolo any opinion on that?
> 
> Also retranslation has a cost (actually on MIPS we spend more time in
> *retranslation* than in translation due to the way the MMU works), we
> should avoid it if we already have to save the CPU state for another
> reason. At least your patch should remove the code saving the CPU state
> when possible if the helper uses retranslation instead.

Ok, I'll remove CPU saving where it is not actually used because of the 
exception.

> > > > This patch passes pointer to the translation block internals to the 
> > > > exception
> > > > handler. It allows correct restoring of the icount value.
> > >
> > > Your patch doesn't do that for all the helpers, for example all the
> > > memory access helpers. It probably improves the situation but therefore
> > > doesn't fix it.
> >
> > Right, I missed these helpers. I'll add them in the next version.
> 
> Except we currently don't provide a way in the softmmu code to provide a
> return address...

Softmmu passes return address into tlb_fill function, which passes it to 
raise_exception.
I also fixed HELPER_LD_ATOMIC and HELPER_ST_ATOMIC to recover PC value.
Do you mean something different?

Pavel Dovgalyuk

Re: [Qemu-devel] [PATCH] MIPS: exceptions handling in icount mode

[Aurelien Jarno]

On 2015-06-15 07:53, Pavel Dovgaluk wrote:
> > From: Aurelien Jarno [mailto:aurel...@aurel32.net]
> > On 2015-06-10 11:33, Pavel Dovgalyuk wrote:
> > > This patch fixes exception handling in MIPS.
> > > MIPS instructions generate several types of exceptions.
> > > When exception is generated, it breaks the execution of the current 
> > > translation
> > > block. Implementation of the exceptions handling in MIPS does not 
> > > correctly
> > > restore icount for the instruction which caused the exception. In most 
> > > cases
> > > icount will be decreased by the value equal to the size of TB.
> > 
> > I don't think it is correct. There is no real point of always doing
> > retranslation for an exception triggered from the helpers, especially
> > when the CPU state has been saved before anyway?
> 
> As you know, icount is processed as follows:
> 
> TB:
> if icount < n then exit
> icount -= n
> instr1
> instr2
> ...
> instrn
> exit
> 
> When one of the instructions initiates an exception, then icount should be 
> restored
> and adjusted number of instructions should be subtracted instead of initial n.
> 
> E.g., tlb_fill function passes retaddr to raise_exception, which allows 
> restoring
> current instructions in TB and correct icount calculation.
> 
> When exception triggered with other function (e.g. by embedding call to 
> helper_raise_exception_err into TB), then PC is not passed as retaddr and
> correct icount is not recovered.
> 
> This behavior leads to incorrect values of timers and non-deterministic 
> execution 
> of the code.

Ok, this therefore doesn't looks something MIPS specific, but rather a
flaw in the icount design. Instead of fixing blindly one target, we
should try to fix it globally, or if not possible at least agree on a
way to fix that for all target and provide the infrastructure for that
(for example provide load/store functions which accept a return
address). Paolo any opinion on that?

Also retranslation has a cost (actually on MIPS we spend more time in
*retranslation* than in translation due to the way the MMU works), we
should avoid it if we already have to save the CPU state for another
reason. At least your patch should remove the code saving the CPU state
when possible if the helper uses retranslation instead.

> > > This patch passes pointer to the translation block internals to the 
> > > exception
> > > handler. It allows correct restoring of the icount value.
> > 
> > Your patch doesn't do that for all the helpers, for example all the
> > memory access helpers. It probably improves the situation but therefore
> > doesn't fix it.
> 
> Right, I missed these helpers. I'll add them in the next version.

Except we currently don't provide a way in the softmmu code to provide a
return address...

Aurelien

-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net

Re: [Qemu-devel] [PATCH] MIPS: exceptions handling in icount mode

[Pavel Dovgaluk]

> From: Aurelien Jarno [mailto:aurel...@aurel32.net]
> On 2015-06-10 11:33, Pavel Dovgalyuk wrote:
> > This patch fixes exception handling in MIPS.
> > MIPS instructions generate several types of exceptions.
> > When exception is generated, it breaks the execution of the current 
> > translation
> > block. Implementation of the exceptions handling in MIPS does not correctly
> > restore icount for the instruction which caused the exception. In most cases
> > icount will be decreased by the value equal to the size of TB.
> 
> I don't think it is correct. There is no real point of always doing
> retranslation for an exception triggered from the helpers, especially
> when the CPU state has been saved before anyway?

As you know, icount is processed as follows:

TB:
if icount < n then exit
icount -= n
instr1
instr2
...
instrn
exit

When one of the instructions initiates an exception, then icount should be 
restored
and adjusted number of instructions should be subtracted instead of initial n.

E.g., tlb_fill function passes retaddr to raise_exception, which allows 
restoring
current instructions in TB and correct icount calculation.

When exception triggered with other function (e.g. by embedding call to 
helper_raise_exception_err into TB), then PC is not passed as retaddr and
correct icount is not recovered.

This behavior leads to incorrect values of timers and non-deterministic 
execution 
of the code.

> > This patch passes pointer to the translation block internals to the 
> > exception
> > handler. It allows correct restoring of the icount value.
> 
> Your patch doesn't do that for all the helpers, for example all the
> memory access helpers. It probably improves the situation but therefore
> doesn't fix it.

Right, I missed these helpers. I'll add them in the next version.

> From my point of view, it looks like the problem is actually elsewhere
> in the common icount code. Do we know if it works correctly on other
> emulated architectures?

This problem can be solved only if someone passes PC to cpu_restore_state 
function.
It cannot be done without altering exceptions processing of the targets.

Other architectures have bugs in icount processing in case of the exceptions.
I will fix them and submit as separate patches.

> Also do you have a quick example to reproduce
> the issue?

I have no quick example, because I've got this situation in the middle 
of booting Linux process. It is hard to catch, because in most cases 
this bug does not affect the guest behavior.

> 
> 
> > Signed-off-by: Pavel Dovgalyuk 
> > ---
> >  target-mips/cpu.h|   28 +
> >  target-mips/msa_helper.c |5 +++-
> >  target-mips/op_helper.c  |   52 
> > +++---
> >  target-mips/translate.c  |2 ++
> >  4 files changed, 45 insertions(+), 42 deletions(-)
> 
> [ snip ]
> 
> > diff --git a/target-mips/translate.c b/target-mips/translate.c
> > index fd063a2..9c2ff7c 100644
> > --- a/target-mips/translate.c
> > +++ b/target-mips/translate.c
> > @@ -1675,6 +1675,7 @@ generate_exception_err (DisasContext *ctx, int excp, 
> > int err)
> >  TCGv_i32 terr = tcg_const_i32(err);
> >  save_cpu_state(ctx, 1);
> >  gen_helper_raise_exception_err(cpu_env, texcp, terr);
> > +ctx->bstate = BS_STOP;
> >  tcg_temp_free_i32(terr);
> >  tcg_temp_free_i32(texcp);
> >  }
> > @@ -1684,6 +1685,7 @@ generate_exception (DisasContext *ctx, int excp)
> >  {
> >  save_cpu_state(ctx, 1);
> >  gen_helper_0e0i(raise_exception, excp);
> > +ctx->bstate = BS_STOP;
> >  }
> >
> 
> Why do we need to stop the translation here? The exception might be
> conditional (for example for ADDU or SUBU).

Right, it would be better to add another helper, which recovers the PC.

Pavel Dovgalyuk

Re: [Qemu-devel] [PATCH] MIPS: exceptions handling in icount mode

[Aurelien Jarno]

On 2015-06-10 11:33, Pavel Dovgalyuk wrote:
> This patch fixes exception handling in MIPS.
> MIPS instructions generate several types of exceptions.
> When exception is generated, it breaks the execution of the current 
> translation
> block. Implementation of the exceptions handling in MIPS does not correctly
> restore icount for the instruction which caused the exception. In most cases
> icount will be decreased by the value equal to the size of TB.

I don't think it is correct. There is no real point of always doing
retranslation for an exception triggered from the helpers, especially
when the CPU state has been saved before anyway?

> This patch passes pointer to the translation block internals to the exception
> handler. It allows correct restoring of the icount value.

Your patch doesn't do that for all the helpers, for example all the
memory access helpers. It probably improves the situation but therefore
doesn't fix it.

From my point of view, it looks like the problem is actually elsewhere
in the common icount code. Do we know if it works correctly on other
emulated architectures? Also do you have a quick example to reproduce
the issue?


> Signed-off-by: Pavel Dovgalyuk 
> ---
>  target-mips/cpu.h|   28 +
>  target-mips/msa_helper.c |5 +++-
>  target-mips/op_helper.c  |   52 
> +++---
>  target-mips/translate.c  |2 ++
>  4 files changed, 45 insertions(+), 42 deletions(-)

[ snip ]

> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index fd063a2..9c2ff7c 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -1675,6 +1675,7 @@ generate_exception_err (DisasContext *ctx, int excp, 
> int err)
>  TCGv_i32 terr = tcg_const_i32(err);
>  save_cpu_state(ctx, 1);
>  gen_helper_raise_exception_err(cpu_env, texcp, terr);
> +ctx->bstate = BS_STOP;
>  tcg_temp_free_i32(terr);
>  tcg_temp_free_i32(texcp);
>  }
> @@ -1684,6 +1685,7 @@ generate_exception (DisasContext *ctx, int excp)
>  {
>  save_cpu_state(ctx, 1);
>  gen_helper_0e0i(raise_exception, excp);
> +ctx->bstate = BS_STOP;
>  }
>  

Why do we need to stop the translation here? The exception might be
conditional (for example for ADDU or SUBU).

-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net

[Qemu-devel] [PATCH] MIPS: exceptions handling in icount mode

[Pavel Dovgalyuk]

This patch fixes exception handling in MIPS.
MIPS instructions generate several types of exceptions.
When exception is generated, it breaks the execution of the current translation
block. Implementation of the exceptions handling in MIPS does not correctly
restore icount for the instruction which caused the exception. In most cases
icount will be decreased by the value equal to the size of TB.
This patch passes pointer to the translation block internals to the exception
handler. It allows correct restoring of the icount value.

Signed-off-by: Pavel Dovgalyuk 
---
 target-mips/cpu.h|   28 +
 target-mips/msa_helper.c |5 +++-
 target-mips/op_helper.c  |   52 +++---
 target-mips/translate.c  |2 ++
 4 files changed, 45 insertions(+), 42 deletions(-)

diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index f9d2b4c..70ba39a 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -1015,4 +1015,32 @@ static inline void cpu_mips_store_cause(CPUMIPSState 
*env, target_ulong val)
 }
 #endif
 
+static inline void QEMU_NORETURN do_raise_exception_err(CPUMIPSState *env,
+uint32_t exception,
+int error_code,
+uintptr_t pc)
+{
+CPUState *cs = CPU(mips_env_get_cpu(env));
+
+if (exception < EXCP_SC) {
+qemu_log("%s: %d %d\n", __func__, exception, error_code);
+}
+cs->exception_index = exception;
+env->error_code = error_code;
+
+if (pc) {
+/* now we have a real cpu fault */
+cpu_restore_state(cs, pc);
+}
+
+cpu_loop_exit(cs);
+}
+
+static inline void QEMU_NORETURN do_raise_exception(CPUMIPSState *env,
+uint32_t exception,
+uintptr_t pc)
+{
+do_raise_exception_err(env, exception, 0, pc);
+}
+
 #endif /* !defined (__MIPS_CPU_H__) */
diff --git a/target-mips/msa_helper.c b/target-mips/msa_helper.c
index 26ffdc7..f7bc710 100644
--- a/target-mips/msa_helper.c
+++ b/target-mips/msa_helper.c
@@ -1352,7 +1352,7 @@ void helper_msa_ctcmsa(CPUMIPSState *env, target_ulong 
elm, uint32_t cd)
 /* check exception */
 if ((GET_FP_ENABLE(env->active_tc.msacsr) | FP_UNIMPLEMENTED)
 & GET_FP_CAUSE(env->active_tc.msacsr)) {
-helper_raise_exception(env, EXCP_MSAFPE);
+do_raise_exception(env, EXCP_MSAFPE, GETPC());
 }
 break;
 }
@@ -1512,7 +1512,8 @@ static inline void check_msacsr_cause(CPUMIPSState *env)
 UPDATE_FP_FLAGS(env->active_tc.msacsr,
 GET_FP_CAUSE(env->active_tc.msacsr));
 } else {
-helper_raise_exception(env, EXCP_MSAFPE);
+/* Will work only when check_msacsr_cause is actually inlined */
+do_raise_exception(env, EXCP_MSAFPE, GETPC());
 }
 }
 
diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index 73a8e45..1b798a2 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -30,34 +30,6 @@ static inline void cpu_mips_tlb_flush (CPUMIPSState *env, 
int flush_global);
 /*/
 /* Exceptions processing helpers */
 
-static inline void QEMU_NORETURN do_raise_exception_err(CPUMIPSState *env,
-uint32_t exception,
-int error_code,
-uintptr_t pc)
-{
-CPUState *cs = CPU(mips_env_get_cpu(env));
-
-if (exception < EXCP_SC) {
-qemu_log("%s: %d %d\n", __func__, exception, error_code);
-}
-cs->exception_index = exception;
-env->error_code = error_code;
-
-if (pc) {
-/* now we have a real cpu fault */
-cpu_restore_state(cs, pc);
-}
-
-cpu_loop_exit(cs);
-}
-
-static inline void QEMU_NORETURN do_raise_exception(CPUMIPSState *env,
-uint32_t exception,
-uintptr_t pc)
-{
-do_raise_exception_err(env, exception, 0, pc);
-}
-
 void helper_raise_exception_err(CPUMIPSState *env, uint32_t exception,
 int error_code)
 {
@@ -309,7 +281,7 @@ target_ulong helper_##name(CPUMIPSState *env, target_ulong 
arg, int mem_idx)  \
 { \
 if (arg & almask) {   \
 env->CP0_BadVAddr = arg;  \
-helper_raise_exception(env, EXCP_AdEL);   \
+do_raise_exception(env, EXCP_AdEL, GETPC());  \
 }   

Re: [Qemu-devel] [PATCH] mips: Correctly save/restore the FP flush-to-zero state

[Maciej W. Rozycki]

On Fri, 5 Dec 2014, Leon Alrae wrote:

> >  I gave it a thought before making this change and concluded it would be 
> > the lesser evil (plus loudly manifesting and easily correctable) if 
> > someone accidentally makes QEMU refuse to load older images where in 
> > fact no compatibility issue exists, than if the reverse is the case, 
> > that is older incompatible images are accepted where they should not 
> > (causing a silent misinterpretation of data), simply because someone 
> > missed the need to change the condition in addition to bumping up 
> > CPU_SAVE_VERSION.  WDYT?
> 
> Finally I got round to reviewing v2 of this patch. Above sounds
> reasonable for me and I haven't seen any objections.
> Hopefully we will convert it into VMState during 2.3 development (Peter,
> thanks for pointing at the commit, it will certainly help).
> 
> Maciej, the only question I have is why you removed only some of "if
> (version_id >= X)" lines in machine.c?

 Argh, because I missed them.  I regenerated the original change and 
didn't think of checking if additional conditionals haven't been added 
upstream since the inception of the original change.  Sorry about that.  
I'll review all the relevant patches, see what has to be updated and 
resend.  Thanks for pointing this out.

 I'll give a priority to the 2008-NaN change though whose final updated 
version has just passed regression-testing (13+hrs per run).  It 
actually updates this save/restore logic here again and introduces the 
CPU_SAVE_VERSION_OLDEST_SUPPORTED macro I proposed here, to support 
older legacy-NaN images that are compatible with the 2008-NaN update.

  Maciej

Re: [Qemu-devel] [PATCH] mips: Correctly save/restore the FP flush-to-zero state

[Leon Alrae]

On 12/11/2014 18:58, Maciej W. Rozycki wrote:
> On Wed, 12 Nov 2014, Peter Maydell wrote:
> 
>>> @@ -208,12 +206,12 @@ int cpu_load(QEMUFile *f, void *opaque,
>>>  MIPSCPU *cpu = mips_env_get_cpu(env);
>>>  int i;
>>>
>>> -if (version_id < 3) {
>>> +if (version_id != CPU_SAVE_VERSION) {
>>>  return -EINVAL;
>>>  }
>>
>> Shouldn't this read "if (version_id < 6)" ?
>> Otherwise next time somebody bumps the CPU_SAVE_VERSION it
>> will give another migration compatibility break without that
>> being very obvious.
> 
>  I gave it a thought before making this change and concluded it would be 
> the lesser evil (plus loudly manifesting and easily correctable) if 
> someone accidentally makes QEMU refuse to load older images where in 
> fact no compatibility issue exists, than if the reverse is the case, 
> that is older incompatible images are accepted where they should not 
> (causing a silent misinterpretation of data), simply because someone 
> missed the need to change the condition in addition to bumping up 
> CPU_SAVE_VERSION.  WDYT?

Finally I got round to reviewing v2 of this patch. Above sounds
reasonable for me and I haven't seen any objections.
Hopefully we will convert it into VMState during 2.3 development (Peter,
thanks for pointing at the commit, it will certainly help).

Maciej, the only question I have is why you removed only some of "if
(version_id >= X)" lines in machine.c?

Regards,
Leon

Re: [Qemu-devel] [PATCH] mips: Fix the 64-bit case for microMIPS MOVE16 and MOVEP

[Maciej W. Rozycki]

On Mon, 24 Nov 2014, Leon Alrae wrote:

> All the patches up to this one have been applied to mips-next branch
> (available at git://github.com/lalrae/qemu.git), thanks. I'll go through
> the remaining soon.

 Thanks.  I am now back from a week's vacation and will continue posting 
outstanding changes.

 There will be changes made to generic code, specifically soft-float, 
and consequently other platforms' code, to suit the MIPS implementation 
of IEEE 754-2008 NaN handling recommendation.  Regrettably I see the 
delivery of the 2.2 release has been delayed and I do hope the new 
estimate of Dec 5th will stand so that the changes can make their way to 
trunk in a timely manner.

 I have some other stuff beyond 2008-NaN support as well, but I'll be 
giving the latter a priority as I know you look forward to seeing it and 
the rest is valuable, but a bit less important (and furthermore it 
relies on some changes to ABI configuration that we may need to discuss 
before we find a satisfactory solution).

  Maciej

Re: [Qemu-devel] [PATCH] mips: kvm: do not use get_clock()

[James Hogan]

On 26/11/14 14:01, Paolo Bonzini wrote:
> Use the external qemu-timer API instead.
> 
> Signed-off-by: Paolo Bonzini 

Acked-by: James Hogan 

Thanks
James

> ---
>  target-mips/kvm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target-mips/kvm.c b/target-mips/kvm.c
> index 97fd51a..a761ea5 100644
> --- a/target-mips/kvm.c
> +++ b/target-mips/kvm.c
> @@ -439,7 +439,7 @@ static void kvm_mips_update_state(void *opaque, int 
> running, RunState state)
>  }
>  } else {
>  /* Set clock restore time to now */
> -count_resume = get_clock();
> +count_resume = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
>  ret = kvm_mips_put_one_reg64(cs, KVM_REG_MIPS_COUNT_RESUME,
>   &count_resume);
>  if (ret < 0) {
> 



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH] mips: kvm: do not use get_clock()

[Paolo Bonzini]

Use the external qemu-timer API instead.

Signed-off-by: Paolo Bonzini 
---
 target-mips/kvm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-mips/kvm.c b/target-mips/kvm.c
index 97fd51a..a761ea5 100644
--- a/target-mips/kvm.c
+++ b/target-mips/kvm.c
@@ -439,7 +439,7 @@ static void kvm_mips_update_state(void *opaque, int 
running, RunState state)
 }
 } else {
 /* Set clock restore time to now */
-count_resume = get_clock();
+count_resume = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
 ret = kvm_mips_put_one_reg64(cs, KVM_REG_MIPS_COUNT_RESUME,
  &count_resume);
 if (ret < 0) {
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] mips: Fix the 64-bit case for microMIPS MOVE16 and MOVEP

[Leon Alrae]

On 12/11/2014 15:21, Maciej W. Rozycki wrote:
> Fix microMIPS MOVE16 and MOVEP instructions on 64-bit processors by 
> using register addition operations.
> 
> This copies the approach taken with MIPS16 MOVE instructions (I8_MOV32R 
> and I8_MOVR32 opcodes) and follows the observation that OPC_ADDU expands 
> to tcg_gen_mov_tl whenever `rt' is 0 and `rs' is not, therefore copying 
> `rs' to `rd' verbatim.  This is not the case with OPC_ADDIU where a 
> sign-extension from bit #31 is made, unless in the uninteresting case of 
> `rs' being 0, losing the upper 32 bits of the value copied for any 
> proper 64-bit values.
> 
> This also serves as an optimization as one op is produced in generated 
> code rather than two (again, unless `rs' is 0, where it doesn't change 
> anything).
> 
> Signed-off-by: Maciej W. Rozycki 
> ---
>  This is rather obvious, but I also pushed it through full bare-iron GCC 
> regression testing with an o32 big-endian microMIPS multilib.  That 
> includes several instances of both instructions.  No changes in results 
> were observed with the patch applied compared to the original version.
> 
>  I wonder if all these move operations shouldn't actually be switched to 
> using OPC_OR that is agnostic to the machine word size regardless of the 
> operand selection.  But that is something to consider separately.
> 
>  So meanwhile, please apply.
> 
>   Maciej
> 
> qemu-umips-move.diff
> Index: qemu-git-trunk/target-mips/translate.c
> ===
> --- qemu-git-trunk.orig/target-mips/translate.c   2014-11-02 
> 17:57:16.998924336 +
> +++ qemu-git-trunk/target-mips/translate.c2014-11-02 17:57:19.498930155 
> +
> @@ -13492,8 +13492,8 @@ static int decode_micromips_opc (CPUMIPS
>  rs = rs_rt_enc[enc_rs];
>  rt = rs_rt_enc[enc_rt];
>  
> -gen_arith_imm(ctx, OPC_ADDIU, rd, rs, 0);
> -gen_arith_imm(ctx, OPC_ADDIU, re, rt, 0);
> +gen_arith(ctx, OPC_ADDU, rd, rs, 0);
> +gen_arith(ctx, OPC_ADDU, re, rt, 0);
>  }
>  break;
>  case LBU16:
> @@ -13574,7 +13574,7 @@ static int decode_micromips_opc (CPUMIPS
>  int rd = uMIPS_RD5(ctx->opcode);
>  int rs = uMIPS_RS5(ctx->opcode);
>  
> -gen_arith_imm(ctx, OPC_ADDIU, rd, rs, 0);
> +gen_arith(ctx, OPC_ADDU, rd, rs, 0);
>  }
>  break;
>  case ANDI16:

Reviewed-by: Leon Alrae 

All the patches up to this one have been applied to mips-next branch
(available at git://github.com/lalrae/qemu.git), thanks. I'll go through
the remaining soon.

Regards,
Leon

Re: [Qemu-devel] [PATCH] mips: Correct the writes to CP0 Status and Cause registers via gdbstub

[Leon Alrae]

Hi Maciej,

On 10/11/2014 13:46, Maciej W. Rozycki wrote:
> qemu-mips-status.diff
> Index: qemu-git-trunk/target-mips/cpu.h
> ===
> --- qemu-git-trunk.orig/target-mips/cpu.h 2014-11-09 23:44:32.0 
> +
> +++ qemu-git-trunk/target-mips/cpu.h  2014-11-09 23:49:54.997846651 +

Could you please include diffstat when you generate patches? (git
format-patch adds it by default). It makes review much easier -
especially of longer patches - as it immediately tells which files and
how many lines have been modified before starting looking at the actual
changes.

Thanks,
Leon

Re: [Qemu-devel] [PATCH] mips: Correct the handling of writes to CP0.Status for MIPSr6

[Leon Alrae]

On 10/11/2014 13:45, Maciej W. Rozycki wrote:
> Correct these issues with the handling of CP0.Status for MIPSr6:
> 
> * only ignore the bit pattern of 0b11 on writes to CP0.Status.KSU, that 
>   is for processors that do implement Supervisor Mode, let the bit
>   pattern be written to CP0.Status.UM:R0 freely (of course the value 
>   written to read-only CP0.Status.R0 will be discarded anyway); this is 
>   in accordance to the relevant architecture specification[1],
> 
> * check the newly written pattern rather than the current contents of 
>   CP0.Status for the KSU bits being 0b11,
> 
> * use meaningful macro names to refer to CP0.Status bits rather than 
>   magic numbers.
> 
> References:
> 
> [1] "MIPS Architecture For Programmers, Volume III: MIPS64 / microMIPS64
> Privileged Resource Architecture", MIPS Technologies, Inc., Document 
> Number: MD00091, Revision 6.00, March 31, 2014, Table 9.45 "Status 
> Register Field Descriptions", pp. 210-211.
> 
> Signed-off-by: Maciej W. Rozycki 
> ---
> Leon,
> 
>  Noticed in porting the next change I'm going to post.  NB I have no 
> reasonable way to do run-time checks of an r6 configuration, so please 
> double check this works for you even though I believe it is obviously 
> correct; I did check CP0.Status.KSU rejects the pattern of 0b11 via GDB 
> on MIPS64R6-generic with the aid of the next change.
> 
>  I suggest making it a policy not to accept new code using magic 
> numbers.  Then the existing places can be gradually identified and 
> corrected.
> 
>  Please apply.
> 
>  Thanks,
> 
>   Maciej
> 
> qemu-mips-r6-status-r0.diff
> Index: qemu-git-trunk/target-mips/op_helper.c
> ===
> --- qemu-git-trunk.orig/target-mips/op_helper.c   2014-11-09 
> 23:44:45.467759913 +
> +++ qemu-git-trunk/target-mips/op_helper.c2014-11-09 23:45:27.977531070 
> +
> @@ -1423,10 +1423,12 @@ void helper_mtc0_status(CPUMIPSState *en
>  uint32_t mask = env->CP0_Status_rw_bitmask;
>  
>  if (env->insn_flags & ISA_MIPS32R6) {
> -if (extract32(env->CP0_Status, CP0St_KSU, 2) == 0x3) {
> +bool has_supervisor = extract32(mask, CP0St_KSU, 2) == 0x3;
> +
> +if (has_supervisor && extract32(arg1, CP0St_KSU, 2) == 0x3) {
>  mask &= ~(3 << CP0St_KSU);
>  }
> -mask &= ~(0x0018 & arg1);
> +mask &= ~(((1 << CP0St_SR) | (1 << CP0St_NMI)) & arg1);
>  }
>  
>  val = arg1 & mask;
> 

Thanks for fixing and cleaning this up.

Reviewed-by: Leon Alrae 

Re: [Qemu-devel] [PATCH] mips: Enable vectored interrupt support for the 74Kf CPU

[Leon Alrae]

On 04/11/2014 15:42, Maciej W. Rozycki wrote:
> Enable vectored interrupt support for the 74Kf CPU, reflecting hardware.
> 
> Signed-off-by: Maciej W. Rozycki 
> ---
> qemu-mips-config-74k-vint.diff
> Index: qemu-git-trunk/target-mips/translate_init.c
> ===
> --- qemu-git-trunk.orig/target-mips/translate_init.c  2014-11-04 
> 03:39:48.458972962 +
> +++ qemu-git-trunk/target-mips/translate_init.c   2014-11-04 
> 03:43:15.479004225 +
> @@ -331,7 +331,7 @@ static const mips_def_t mips_defs[] =
> (1 << CP0C1_CA),
>  .CP0_Config2 = MIPS_CONFIG2,
>  .CP0_Config3 = MIPS_CONFIG3 | (1 << CP0C3_DSP2P) | (1 << CP0C3_DSPP) 
> |
> -   (0 << CP0C3_VInt),
> +   (1 << CP0C3_VInt),
>  .CP0_LLAddr_rw_bitmask = 0,
>  .CP0_LLAddr_shift = 4,
>  .SYNCI_Step = 32,
> 

Reviewed-by: Leon Alrae 

Re: [Qemu-devel] [PATCH] mips/gdbstub: Make CP1.FIR read-only here too

[Leon Alrae]

On 03/11/2014 18:51, Maciej W. Rozycki wrote:
> CP1.FIR is read-only in hardware so gdbstub must respect it.  We already 
> respect it for CTC1 instructions, so do it here too.
> 
> Signed-off-by: Maciej W. Rozycki 
> ---
> Not much to say about it here.  Please apply.
> 
>   Maciej
> 
> qemu-mips-fir.diff
> Index: qemu-git-trunk/target-mips/gdbstub.c
> ===
> --- qemu-git-trunk.orig/target-mips/gdbstub.c 2014-11-02 17:51:33.458968203 
> +
> +++ qemu-git-trunk/target-mips/gdbstub.c  2014-11-02 17:51:35.958924223 
> +
> @@ -112,7 +112,7 @@ int mips_cpu_gdb_write_register(CPUState
>  RESTORE_ROUNDING_MODE;
>  break;
>  case 71:
> -env->active_fpu.fcr0 = tmp;
> +/* FIR is read-only.  Ignore writes.  */
>  break;
>  }
>  return sizeof(target_ulong);
> 

Reviewed-by: Leon Alrae 

Re: [Qemu-devel] [PATCH] mips/gdbstub: Correct the handling of register #72 on writes

[Leon Alrae]

On 03/11/2014 18:47, Maciej W. Rozycki wrote:
> Fix an off-by-one error in `mips_cpu_gdb_write_register' for register 
> #72 that is handled further down in that function rather than here, 
> matching how `mips_cpu_gdb_read_register' handles it.  This register 
> slot is a fake anyway, there's nothing in hardware that corresponds to 
> it.
> 
> Signed-off-by: Maciej W. Rozycki 
> ---
>  I have a further change down the queue to clean up 
> `mips_cpu_gdb_read_register' and `mips_cpu_gdb_write_register' and make 
> them more consistent with respect to each other as far as the handling 
> of FP registers is concerned.  For now please apply this obvious change.  
> Thanks.
> 
>   Maciej
> 
> qemu-mips-fpreg72.diff
> Index: qemu-git-trunk/target-mips/gdbstub.c
> ===
> --- qemu-git-trunk.orig/target-mips/gdbstub.c 2013-07-29 11:23:07.048742983 
> +0100
> +++ qemu-git-trunk/target-mips/gdbstub.c  2014-10-27 04:17:19.159003270 
> +
> @@ -97,7 +97,7 @@ int mips_cpu_gdb_write_register(CPUState
>  return sizeof(target_ulong);
>  }
>  if (env->CP0_Config1 & (1 << CP0C1_FP)
> -&& n >= 38 && n < 73) {
> +&& n >= 38 && n < 72) {
>  if (n < 70) {
>  if (env->CP0_Status & (1 << CP0St_FR)) {
>  env->active_fpu.fpr[n - 38].d = tmp;
> 

Reviewed-by: Leon Alrae 

Re: [Qemu-devel] [PATCH] mips: Fix the 64-bit case for microMIPS MOVE16 and MOVEP

[Maciej W. Rozycki]

On Thu, 13 Nov 2014, Leon Alrae wrote:

> It might be a good idea to split these changes into separate patches to
> have more precise indication about touched subsystem (even though all
> the changes were done in MIPS context). For example "target-mips" and
> "linux-user" rather than just "mips".

 I split changes as applicable wherever I can, but at least some of 
these changes cannot be split without losing the sense of the change or 
even without causing compilation breakage.  Well, I plan to submit those 
last, so it'll be a while yet and we can think about it when the time 
comes.  Thanks for your input.

  Maciej

Re: [Qemu-devel] [PATCH] mips: Fix the 64-bit case for microMIPS MOVE16 and MOVEP

[Leon Alrae]

On 12/11/2014 18:46, Maciej W. Rozycki wrote:
> On Wed, 12 Nov 2014, Andreas Färber wrote:
> 
>> Please consistently use "target-mips: " when that's what you're
>> touching. (For hw/mips/ it's less consistent what to use.)
> 
>  Sure.  What about MIPS changes that span files contained within 
> target-mips/ and elsewhere?  I have such changes in my queue.

It might be a good idea to split these changes into separate patches to
have more precise indication about touched subsystem (even though all
the changes were done in MIPS context). For example "target-mips" and
"linux-user" rather than just "mips".

Regards,
Leon

Re: [Qemu-devel] [PATCH] mips: Correctly save/restore the FP flush-to-zero state

[Maciej W. Rozycki]

On Wed, 12 Nov 2014, Peter Maydell wrote:

> > @@ -208,12 +206,12 @@ int cpu_load(QEMUFile *f, void *opaque,
> >  MIPSCPU *cpu = mips_env_get_cpu(env);
> >  int i;
> >
> > -if (version_id < 3) {
> > +if (version_id != CPU_SAVE_VERSION) {
> >  return -EINVAL;
> >  }
> 
> Shouldn't this read "if (version_id < 6)" ?
> Otherwise next time somebody bumps the CPU_SAVE_VERSION it
> will give another migration compatibility break without that
> being very obvious.

 I gave it a thought before making this change and concluded it would be 
the lesser evil (plus loudly manifesting and easily correctable) if 
someone accidentally makes QEMU refuse to load older images where in 
fact no compatibility issue exists, than if the reverse is the case, 
that is older incompatible images are accepted where they should not 
(causing a silent misinterpretation of data), simply because someone 
missed the need to change the condition in addition to bumping up 
CPU_SAVE_VERSION.  WDYT?

 Besides if we go with your suggestion after all, then I think it should 
be:

if (version_id < CPU_SAVE_OLDEST_SUPPORTED_VERSION) {

or suchlike as a hardcoded constant somewhere in the middle of code is 
too easy to miss.  Then the two constants can be maintained in a single 
place.

> As a longer-term cleanup I would highly recommend converting
> the MIPS machine.c to use VMState structs to define its
> migration (this is likely to imply another compatibility
> break, so if you have any plans for supporting cross-QEMU-version
> migration in future you should definitely do the conversion
> before that point). The commit where we did this for ARM was
> 3cc1d20823 which gives you an idea of how the conversion works.
> MIPS, CRIS and SPARC are the only three remaining guest CPUs
> still using the old-style by-hand save/restore, so it would
> be good to finally complete that transition.

 Thanks for the suggestion, it certainly looks like the right direction 
to me.  I need to offload the oustanding stuff first though.

  Maciej

Re: [Qemu-devel] [PATCH] mips: Fix the 64-bit case for microMIPS MOVE16 and MOVEP

[Maciej W. Rozycki]

On Wed, 12 Nov 2014, Andreas Färber wrote:

> Please consistently use "target-mips: " when that's what you're
> touching. (For hw/mips/ it's less consistent what to use.)

 Sure.  What about MIPS changes that span files contained within 
target-mips/ and elsewhere?  I have such changes in my queue.

  Maciej

Re: [Qemu-devel] [PATCH] mips: Fix the 64-bit case for microMIPS MOVE16 and MOVEP

[Andreas Färber]

Hi Maciej,

Please consistently use "target-mips: " when that's what you're
touching. (For hw/mips/ it's less consistent what to use.)

Leon, please sanitize subjects before sending them out, it makes them
easier to skim in git-log and cgit.

Thanks,
Andreas

-- 
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 21284 AG Nürnberg

Re: [Qemu-devel] [PATCH] mips: Correctly save/restore the FP flush-to-zero state

[Peter Maydell]

On 12 November 2014 16:07, Maciej W. Rozycki  wrote:
> Fix the FP state save/restore operations by saving the `flush_to_zero'
> rather than the `float_detect_tininess' setting.  There is no provision
> for the latter in MIPS hardware, whereas the former is controlled by the
> CP1.FCSR.FS bit.  As a result all the older saved state images are
> invalid as they do not restore the FP state corresponding to the
> CP1.FCSR.FS bit and may execute differently when resumed compared to the
> case where no save/restore operations have ever been made.  Therefore
> reject any such older images too and do not allow them to be loaded.

> @@ -208,12 +206,12 @@ int cpu_load(QEMUFile *f, void *opaque,
>  MIPSCPU *cpu = mips_env_get_cpu(env);
>  int i;
>
> -if (version_id < 3) {
> +if (version_id != CPU_SAVE_VERSION) {
>  return -EINVAL;
>  }

Shouldn't this read "if (version_id < 6)" ?
Otherwise next time somebody bumps the CPU_SAVE_VERSION it
will give another migration compatibility break without that
being very obvious.

As a longer-term cleanup I would highly recommend converting
the MIPS machine.c to use VMState structs to define its
migration (this is likely to imply another compatibility
break, so if you have any plans for supporting cross-QEMU-version
migration in future you should definitely do the conversion
before that point). The commit where we did this for ARM was
3cc1d20823 which gives you an idea of how the conversion works.
MIPS, CRIS and SPARC are the only three remaining guest CPUs
still using the old-style by-hand save/restore, so it would
be good to finally complete that transition.

thanks
-- PMM

[Qemu-devel] [PATCH] mips: Correctly save/restore the FP flush-to-zero state

[Maciej W. Rozycki]

Fix the FP state save/restore operations by saving the `flush_to_zero' 
rather than the `float_detect_tininess' setting.  There is no provision 
for the latter in MIPS hardware, whereas the former is controlled by the 
CP1.FCSR.FS bit.  As a result all the older saved state images are 
invalid as they do not restore the FP state corresponding to the 
CP1.FCSR.FS bit and may execute differently when resumed compared to the 
case where no save/restore operations have ever been made.  Therefore 
reject any such older images too and do not allow them to be loaded.

According to the architecture manual[1][2]:

"Flush to Zero (Flush Subnormals).
[...]
Encoding Meaning
 0Input subnormal values and tiny non-
  zero results are not altered.  Unimple-
  mented Operation Exception may be
  signaled as needed.
 1When FS is one, subnormal results are
  flushed to zero.  The Unimplemented
  Operation Exception is NOT signalled
  for this reason.
 Every tiny non-zero result is
  replaced with zero of the same sign.
 Prior to Release 5 it is implementa-
  tion dependent whether subnormal
  operand values are flushed to zero
  before the operation is carried out.
 As of Release 5 every input subnor-
  mal value is replaced with zero of the
  same sign."

References:

[1] "MIPS Architecture For Programmers, Volume I-A: Introduction to the 
MIPS32 Architecture", MIPS Technologies, Inc., Document Number: 
MD00082, Revision 5.03, Sept. 9, 2013, Table 5.7 "FCSR Register 
Field Descriptions", p. 81.

[2] "MIPS Architecture For Programmers, Volume I-A: Introduction to the 
MIPS64 Architecture", Imagination Technologies, Inc., Document 
Number: MD00083, Revision 6.00, March 31, 2014, Table 6.7 "FCSR 
Register Field Descriptions", p. 93.

Signed-off-by: Maciej W. Rozycki 
---
 Hopefully obvious.  Please apply.

  Maciej
qemu-mips-fp_status.diff
Index: qemu-git-trunk/target-mips/cpu.h
===
--- qemu-git-trunk.orig/target-mips/cpu.h   2014-11-12 07:38:02.077674697 
+
+++ qemu-git-trunk/target-mips/cpu.h2014-11-12 07:38:07.577674675 +
@@ -615,7 +615,7 @@ void mips_cpu_list (FILE *f, fprintf_fun
 extern void cpu_wrdsp(uint32_t rs, uint32_t mask_num, CPUMIPSState *env);
 extern uint32_t cpu_rddsp(uint32_t mask_num, CPUMIPSState *env);
 
-#define CPU_SAVE_VERSION 5
+#define CPU_SAVE_VERSION 6
 
 /* MMU modes definitions. We carefully match the indices with our
hflags layout. */
Index: qemu-git-trunk/target-mips/machine.c
===
--- qemu-git-trunk.orig/target-mips/machine.c   2014-11-12 05:38:50.309021048 
+
+++ qemu-git-trunk/target-mips/machine.c2014-11-12 07:38:07.577674675 
+
@@ -34,7 +34,7 @@ static void save_fpu(QEMUFile *f, CPUMIP
 
 for(i = 0; i < 32; i++)
 qemu_put_be64s(f, &fpu->fpr[i].d);
-qemu_put_s8s(f, &fpu->fp_status.float_detect_tininess);
+qemu_put_s8s(f, &fpu->fp_status.flush_to_zero);
 qemu_put_s8s(f, &fpu->fp_status.float_rounding_mode);
 qemu_put_s8s(f, &fpu->fp_status.float_exception_flags);
 qemu_put_be32s(f, &fpu->fcr0);
@@ -162,7 +162,7 @@ void cpu_save(QEMUFile *f, void *opaque)
 save_fpu(f, &env->fpus[i]);
 }
 
-static void load_tc(QEMUFile *f, TCState *tc, int version_id)
+static void load_tc(QEMUFile *f, TCState *tc)
 {
 int i;
 
@@ -184,9 +184,7 @@ static void load_tc(QEMUFile *f, TCState
 qemu_get_betls(f, &tc->CP0_TCSchedule);
 qemu_get_betls(f, &tc->CP0_TCScheFBack);
 qemu_get_sbe32s(f, &tc->CP0_Debug_tcstatus);
-if (version_id >= 4) {
-qemu_get_betls(f, &tc->CP0_UserLocal);
-}
+qemu_get_betls(f, &tc->CP0_UserLocal);
 }
 
 static void load_fpu(QEMUFile *f, CPUMIPSFPUContext *fpu)
@@ -195,7 +193,7 @@ static void load_fpu(QEMUFile *f, CPUMIP
 
 for(i = 0; i < 32; i++)
 qemu_get_be64s(f, &fpu->fpr[i].d);
-qemu_get_s8s(f, &fpu->fp_status.float_detect_tininess);
+qemu_get_s8s(f, &fpu->fp_status.flush_to_zero);
 qemu_get_s8s(f, &fpu->fp_status.float_rounding_mode);
 qemu_get_s8s(f, &fpu->fp_status.float_exception_flags);
 qemu_get_be32s(f, &fpu->fcr0);
@@ -208,12 +206,12 @@ int cpu_load(QEMUFile *f, void *opaque, 
 MIPSCPU *cpu = mips_env_get_cpu(env);
 int i;
 
-if (version_id < 3) {
+if (version_id != CPU_SAVE_VERSION) {
 return -EINVAL;
 }
 
 /* Load active TC */
-load_tc(f, &env->active_tc, version_id);
+load_tc(f, &env->active_tc);
 
 /* Load active FPU */
 load_fpu(f, &env->active_fpu);
@@ -328,7 +326,7 @@ int cpu_load(QEMUFile *f, void *opaque, 
 
 /* Load inactive T

[Qemu-devel] [PATCH] mips: Fix the 64-bit case for microMIPS MOVE16 and MOVEP

[Maciej W. Rozycki]

Fix microMIPS MOVE16 and MOVEP instructions on 64-bit processors by 
using register addition operations.

This copies the approach taken with MIPS16 MOVE instructions (I8_MOV32R 
and I8_MOVR32 opcodes) and follows the observation that OPC_ADDU expands 
to tcg_gen_mov_tl whenever `rt' is 0 and `rs' is not, therefore copying 
`rs' to `rd' verbatim.  This is not the case with OPC_ADDIU where a 
sign-extension from bit #31 is made, unless in the uninteresting case of 
`rs' being 0, losing the upper 32 bits of the value copied for any 
proper 64-bit values.

This also serves as an optimization as one op is produced in generated 
code rather than two (again, unless `rs' is 0, where it doesn't change 
anything).

Signed-off-by: Maciej W. Rozycki 
---
 This is rather obvious, but I also pushed it through full bare-iron GCC 
regression testing with an o32 big-endian microMIPS multilib.  That 
includes several instances of both instructions.  No changes in results 
were observed with the patch applied compared to the original version.

 I wonder if all these move operations shouldn't actually be switched to 
using OPC_OR that is agnostic to the machine word size regardless of the 
operand selection.  But that is something to consider separately.

 So meanwhile, please apply.

  Maciej

qemu-umips-move.diff
Index: qemu-git-trunk/target-mips/translate.c
===
--- qemu-git-trunk.orig/target-mips/translate.c 2014-11-02 17:57:16.998924336 
+
+++ qemu-git-trunk/target-mips/translate.c  2014-11-02 17:57:19.498930155 
+
@@ -13492,8 +13492,8 @@ static int decode_micromips_opc (CPUMIPS
 rs = rs_rt_enc[enc_rs];
 rt = rs_rt_enc[enc_rt];
 
-gen_arith_imm(ctx, OPC_ADDIU, rd, rs, 0);
-gen_arith_imm(ctx, OPC_ADDIU, re, rt, 0);
+gen_arith(ctx, OPC_ADDU, rd, rs, 0);
+gen_arith(ctx, OPC_ADDU, re, rt, 0);
 }
 break;
 case LBU16:
@@ -13574,7 +13574,7 @@ static int decode_micromips_opc (CPUMIPS
 int rd = uMIPS_RD5(ctx->opcode);
 int rs = uMIPS_RS5(ctx->opcode);
 
-gen_arith_imm(ctx, OPC_ADDIU, rd, rs, 0);
+gen_arith(ctx, OPC_ADDU, rd, rs, 0);
 }
 break;
 case ANDI16:

[Qemu-devel] [PATCH] mips: Correct the writes to CP0 Status and Cause registers via gdbstub

[Maciej W. Rozycki]

Make writes to CP0.Status and CP0.Cause have the same effect as
executing corresponding MTC0 instructions would in Kernel Mode.  Also
ignore writes in the user emulation mode.

Currently for requests from the GDB stub we write all the bits across
both registers, ignoring any read-only locations, and do not synchronise
the environment to evaluate side effects.  We also write these registers
in the user emulation mode even though a real kernel presents them as
read only.

Signed-off-by: Maciej W. Rozycki 
---
Hi,

 I have verified the correct operation of this patch in the system 
emulation mode, with the 24Kf processor selected, by:

1. Booting Linux and running full glibc and GDB test suites within it, 
   the latter with gdbserver run locally and GDB connecting to it over 
   TCP from the host running QEMU.  This should have executed enough 
   kernel paths to ensure the semantics of the two registers hasn't 
   changed as seen by code emulated.

2. Connecting with GDB through the GDB stub, downloading a simple 
   bare-iron app linked to a KSEG0 address and single-stepping through 
   it; then poking at CP0.Status to switch to User Mode (KSU set to 
   0b10, ERL & EXL cleared) and single-stepping further, reaching the 
   generic exception handler right away due the an AdEL exception 
   (PC set to 0x8184, CP0.Status.EXL set and CP0.Cause.ExcCode set 
   to 0x04) triggered with an instruction fetch from KSEG0 that is not 
   allowed in User Mode.  With the change reverted single-stepping 
   continues as if nothing happened.

The change for the user emulation mode is obvious.

 While testing this change I noticed that memory accesses made through 
the GDB stub are incorrectly validated against the current execution 
mode set in CP0.Status.  This is obviously incorrect, privilege checks 
should be suppressed as if the GDB stub accessed memory from Debug Mode.  
This is a separate bug to fix though.

 It would be great if QEMU stopped at the first exception handler 
instruction too, that is at 0x8180 in this case, as the change in 
the execution flow is not necessarily expected or the effects easily 
determined and it would be good to see the new state before the first 
exception handler instruction has been executed; this is what real JTAG 
hardware probes actually do, at least certainly on the MIPS target and 
IIRC on Freescale Power and Intel x86 too.

 Please apply.

  Maciej

qemu-mips-status.diff
Index: qemu-git-trunk/target-mips/cpu.h
===
--- qemu-git-trunk.orig/target-mips/cpu.h   2014-11-09 23:44:32.0 
+
+++ qemu-git-trunk/target-mips/cpu.h2014-11-09 23:49:54.997846651 +
@@ -904,4 +904,93 @@ static inline void compute_hflags(CPUMIP
 }
 }
 
+#ifndef CONFIG_USER_ONLY
+/* Called for updates to CP0_Status.  */
+static inline void sync_c0_status(CPUMIPSState *env, CPUMIPSState *cpu, int tc)
+{
+int32_t tcstatus, *tcst;
+uint32_t v = cpu->CP0_Status;
+uint32_t cu, mx, asid, ksu;
+uint32_t mask = ((1 << CP0TCSt_TCU3)
+   | (1 << CP0TCSt_TCU2)
+   | (1 << CP0TCSt_TCU1)
+   | (1 << CP0TCSt_TCU0)
+   | (1 << CP0TCSt_TMX)
+   | (3 << CP0TCSt_TKSU)
+   | (0xff << CP0TCSt_TASID));
+
+cu = (v >> CP0St_CU0) & 0xf;
+mx = (v >> CP0St_MX) & 0x1;
+ksu = (v >> CP0St_KSU) & 0x3;
+asid = env->CP0_EntryHi & 0xff;
+
+tcstatus = cu << CP0TCSt_TCU0;
+tcstatus |= mx << CP0TCSt_TMX;
+tcstatus |= ksu << CP0TCSt_TKSU;
+tcstatus |= asid;
+
+if (tc == cpu->current_tc) {
+tcst = &cpu->active_tc.CP0_TCStatus;
+} else {
+tcst = &cpu->tcs[tc].CP0_TCStatus;
+}
+
+*tcst &= ~mask;
+*tcst |= tcstatus;
+compute_hflags(cpu);
+}
+
+static inline void cpu_mips_store_status(CPUMIPSState *env, target_ulong val)
+{
+uint32_t mask = env->CP0_Status_rw_bitmask;
+
+if (env->insn_flags & ISA_MIPS32R6) {
+bool has_supervisor = extract32(mask, CP0St_KSU, 2) == 0x3;
+
+if (has_supervisor && extract32(val, CP0St_KSU, 2) == 0x3) {
+mask &= ~(3 << CP0St_KSU);
+}
+mask &= ~(((1 << CP0St_SR) | (1 << CP0St_NMI)) & val);
+}
+
+env->CP0_Status = (env->CP0_Status & ~mask) | (val & mask);
+if (env->CP0_Config3 & (1 << CP0C3_MT)) {
+sync_c0_status(env, env, env->current_tc);
+} else {
+compute_hflags(env);
+}
+}
+
+static inline void cpu_mips_store_cause(CPUMIPSState *env, target_ulong val)
+{
+uint32_t mask = 0x00C00300;
+uint32_t old = env->CP0_Cause;
+int i;
+
+if (env->insn_flags & ISA_MIPS32R2) {
+mask |= 1 << CP0Ca_DC;
+}
+if (env->insn_flags & ISA_MIPS32R6) {
+mask &= ~((1 << CP0Ca_WP) & val);
+}
+
+env->CP0_Cause = (env->CP0_Cause & ~mask) | (val & mask);
+
+if ((old ^ env->CP0_Cause) & (1 << CP0Ca_DC)) {
+   

[Qemu-devel] [PATCH] mips: Correct the handling of writes to CP0.Status for MIPSr6

[Maciej W. Rozycki]

Correct these issues with the handling of CP0.Status for MIPSr6:

* only ignore the bit pattern of 0b11 on writes to CP0.Status.KSU, that 
  is for processors that do implement Supervisor Mode, let the bit
  pattern be written to CP0.Status.UM:R0 freely (of course the value 
  written to read-only CP0.Status.R0 will be discarded anyway); this is 
  in accordance to the relevant architecture specification[1],

* check the newly written pattern rather than the current contents of 
  CP0.Status for the KSU bits being 0b11,

* use meaningful macro names to refer to CP0.Status bits rather than 
  magic numbers.

References:

[1] "MIPS Architecture For Programmers, Volume III: MIPS64 / microMIPS64
Privileged Resource Architecture", MIPS Technologies, Inc., Document 
Number: MD00091, Revision 6.00, March 31, 2014, Table 9.45 "Status 
Register Field Descriptions", pp. 210-211.

Signed-off-by: Maciej W. Rozycki 
---
Leon,

 Noticed in porting the next change I'm going to post.  NB I have no 
reasonable way to do run-time checks of an r6 configuration, so please 
double check this works for you even though I believe it is obviously 
correct; I did check CP0.Status.KSU rejects the pattern of 0b11 via GDB 
on MIPS64R6-generic with the aid of the next change.

 I suggest making it a policy not to accept new code using magic 
numbers.  Then the existing places can be gradually identified and 
corrected.

 Please apply.

 Thanks,

  Maciej

qemu-mips-r6-status-r0.diff
Index: qemu-git-trunk/target-mips/op_helper.c
===
--- qemu-git-trunk.orig/target-mips/op_helper.c 2014-11-09 23:44:45.467759913 
+
+++ qemu-git-trunk/target-mips/op_helper.c  2014-11-09 23:45:27.977531070 
+
@@ -1423,10 +1423,12 @@ void helper_mtc0_status(CPUMIPSState *en
 uint32_t mask = env->CP0_Status_rw_bitmask;
 
 if (env->insn_flags & ISA_MIPS32R6) {
-if (extract32(env->CP0_Status, CP0St_KSU, 2) == 0x3) {
+bool has_supervisor = extract32(mask, CP0St_KSU, 2) == 0x3;
+
+if (has_supervisor && extract32(arg1, CP0St_KSU, 2) == 0x3) {
 mask &= ~(3 << CP0St_KSU);
 }
-mask &= ~(0x0018 & arg1);
+mask &= ~(((1 << CP0St_SR) | (1 << CP0St_NMI)) & arg1);
 }
 
 val = arg1 & mask;

Re: [Qemu-devel] [PATCH] mips: Set the CP0.Config3.DSP and CP0.Config3.DSP2P bits

[Maciej W. Rozycki]

On Fri, 7 Nov 2014, Leon Alrae wrote:

> >> I was considering making mips32r5-generic less artificial and slowly
> >> evolve it towards some existing MIPS32R5 CPU, for example P5600 (which
> >> supports MSA, but doesn't support DSP ASE). Furthermore, none from the
> >> latest MIPS CPUs supports both ASEs.
> > 
> >  Why not leave mips32r5-generic alone then and add a correct entry for 
> > the P5600 instead?
> 
> Because it is additional configuration which is not specified anywhere
> that has to be maintain and tested. If a user wants to experiment with
> configurations, then it is relatively easy to add new or modify existing
> CPU definitions and this doesn't require any knowledge of QEMU. The only
> clear benefit for me from having a generic core is when new
> architectural feature is introduced and there is no actual CPU
> available. In this case we use generic core to expose new feature to a
> user. But in my opinion it eventually should be replaced by a real CPU
> containing supporting feature.

 I see having generic entries as a way to verify architecture 
conformance, including but not limited to make it possible to validate 
portability of software for which there is no suitable hardware 
available.  This is actually one of the main purposes for processor 
emulators to exist in the first place.  Here for example one could check 
if context switching is implemented correctly in the Linux kernel for a 
processor that implements both the MSA and the DSP register set -- which 
is something that is best done beforehand, before lots of people get in 
trouble once such hardware is decided to be made.

 And I disagree with a claim that such configurations are not specified 
anywhere -- they are, in the architecture specifications that allows 
them.  A good example in the context of the MIPS architecture is the 
semantics of the CP0.Status.MX bit in hardware that has both the DSP and 
the MDMX ASE implemented -- it has been specified very precisely in the 
architecture even though to the best of my knowledge no such hardware 
has actually ever been made.

 Whether the maintenance burden to have these extra configurations 
(pretty low IMO, but it's not my call to decide here) is justified or 
not is another question of course.

  Maciej

Re: [Qemu-devel] [PATCH] mips: Set the CP0.Config3.DSP and CP0.Config3.DSP2P bits

[Leon Alrae]

On 07/11/14 17:36, Maciej W. Rozycki wrote:
> On Fri, 7 Nov 2014, Leon Alrae wrote:
> 
>>>  I have been working with the current trunk, the change applies 
>>> correctly there AFAICT.
>>
>> 55a2201 commit added (1 << CP0C3_MSAP) to CP0_Config3 for
>> mips32r5-generic which is not present on your patch.
> 
>  Indeed, my mistake for some reason.
> 
>>>  I have no objections to changing mips32r5-generic, it is artificial 
>>> anyway.  But what do you mean by DSP and MSA on one CPU having no sense, 
>>> is there a conflict between the two ASEs?
>>
>> I was considering making mips32r5-generic less artificial and slowly
>> evolve it towards some existing MIPS32R5 CPU, for example P5600 (which
>> supports MSA, but doesn't support DSP ASE). Furthermore, none from the
>> latest MIPS CPUs supports both ASEs.
> 
>  Why not leave mips32r5-generic alone then and add a correct entry for 
> the P5600 instead?

Because it is additional configuration which is not specified anywhere
that has to be maintain and tested. If a user wants to experiment with
configurations, then it is relatively easy to add new or modify existing
CPU definitions and this doesn't require any knowledge of QEMU. The only
clear benefit for me from having a generic core is when new
architectural feature is introduced and there is no actual CPU
available. In this case we use generic core to expose new feature to a
user. But in my opinion it eventually should be replaced by a real CPU
containing supporting feature.

Regards,
Leon

Re: [Qemu-devel] [PATCH] mips: Set the CP0.Config3.DSP and CP0.Config3.DSP2P bits

[Maciej W. Rozycki]

On Fri, 7 Nov 2014, Leon Alrae wrote:

> >  I have been working with the current trunk, the change applies 
> > correctly there AFAICT.
> 
> 55a2201 commit added (1 << CP0C3_MSAP) to CP0_Config3 for
> mips32r5-generic which is not present on your patch.

 Indeed, my mistake for some reason.

> >  I have no objections to changing mips32r5-generic, it is artificial 
> > anyway.  But what do you mean by DSP and MSA on one CPU having no sense, 
> > is there a conflict between the two ASEs?
> 
> I was considering making mips32r5-generic less artificial and slowly
> evolve it towards some existing MIPS32R5 CPU, for example P5600 (which
> supports MSA, but doesn't support DSP ASE). Furthermore, none from the
> latest MIPS CPUs supports both ASEs.

 Why not leave mips32r5-generic alone then and add a correct entry for 
the P5600 instead?

  Maciej

Re: [Qemu-devel] [PATCH] mips: Set the CP0.Config3.DSP and CP0.Config3.DSP2P bits

[Leon Alrae]

On 07/11/2014 12:33, Maciej W. Rozycki wrote:
> On Fri, 7 Nov 2014, Leon Alrae wrote:
> 
>> When I've been applying this patch to my mips-next candidate branch for
>> 2.2 I realized that you haven't rebased it onto the recent version where
>> MSA has been added to mips32r5-generic. Now I don't think that having
>> DSP and MSA on one CPU makes sense, therefore what I'm going to do is to
>> change mips32r5-generic part in your patch slightly: instead of setting
>> CP0.Config3.DSP/DSP2P the patch will remove ASE_DSP/DSPR2 insn_flags.
> 
>  I have been working with the current trunk, the change applies 
> correctly there AFAICT.

55a2201 commit added (1 << CP0C3_MSAP) to CP0_Config3 for
mips32r5-generic which is not present on your patch.

> 
>  I have no objections to changing mips32r5-generic, it is artificial 
> anyway.  But what do you mean by DSP and MSA on one CPU having no sense, 
> is there a conflict between the two ASEs?

I was considering making mips32r5-generic less artificial and slowly
evolve it towards some existing MIPS32R5 CPU, for example P5600 (which
supports MSA, but doesn't support DSP ASE). Furthermore, none from the
latest MIPS CPUs supports both ASEs.

Regards,
Leon

Re: [Qemu-devel] [PATCH] mips: Set the CP0.Config3.DSP and CP0.Config3.DSP2P bits

[Maciej W. Rozycki]

On Fri, 7 Nov 2014, Leon Alrae wrote:

> When I've been applying this patch to my mips-next candidate branch for
> 2.2 I realized that you haven't rebased it onto the recent version where
> MSA has been added to mips32r5-generic. Now I don't think that having
> DSP and MSA on one CPU makes sense, therefore what I'm going to do is to
> change mips32r5-generic part in your patch slightly: instead of setting
> CP0.Config3.DSP/DSP2P the patch will remove ASE_DSP/DSPR2 insn_flags.

 I have been working with the current trunk, the change applies 
correctly there AFAICT.

 I have no objections to changing mips32r5-generic, it is artificial 
anyway.  But what do you mean by DSP and MSA on one CPU having no sense, 
is there a conflict between the two ASEs?

  Maciej

Re: [Qemu-devel] [PATCH] mips: Set the CP0.Config3.DSP and CP0.Config3.DSP2P bits

[Leon Alrae]

On 05/11/2014 15:26, Leon Alrae wrote:
> On 04/11/2014 15:41, Maciej W. Rozycki wrote:
>> Set the CP0.Config3.DSP2P bit for the 74kf processor and both that bit 
>> and the CP0.Config3.DSP bit for the artificial mips32r5-generic and 
>> mips64dspr2 processors.  They have the DSPr2 ASE enabled in `insn_flags' 
>> and CPUs that implement that ASE need to have both CP0.Config3.DSP and 
>> CP0.Config3.DSP2P set or software won't detect its presence.
>>
>> Signed-off-by: Maciej W. Rozycki 
>> ---
>> qemu-mips-config-dsp.diff
>> Index: qemu-git-trunk/target-mips/translate_init.c
>> ===
>> --- qemu-git-trunk.orig/target-mips/translate_init.c 2014-11-04 
>> 03:32:21.408100354 +
>> +++ qemu-git-trunk/target-mips/translate_init.c  2014-11-04 
>> 03:39:48.458972962 +
>> @@ -330,7 +330,8 @@ static const mips_def_t mips_defs[] =
>> (0 << CP0C1_DS) | (3 << CP0C1_DL) | (1 << CP0C1_DA) |
>> (1 << CP0C1_CA),
>>  .CP0_Config2 = MIPS_CONFIG2,
>> -.CP0_Config3 = MIPS_CONFIG3 | (0 << CP0C3_VInt) | (1 << CP0C3_DSPP),
>> +.CP0_Config3 = MIPS_CONFIG3 | (1 << CP0C3_DSP2P) | (1 << 
>> CP0C3_DSPP) |
>> +   (0 << CP0C3_VInt),
>>  .CP0_LLAddr_rw_bitmask = 0,
>>  .CP0_LLAddr_shift = 4,
>>  .SYNCI_Step = 32,
>> @@ -396,7 +397,8 @@ static const mips_def_t mips_defs[] =
>> (0 << CP0C1_DS) | (3 << CP0C1_DL) | (1 << CP0C1_DA) |
>> (1 << CP0C1_CA),
>>  .CP0_Config2 = MIPS_CONFIG2,
>> -.CP0_Config3 = MIPS_CONFIG3 | (1U << CP0C3_M),
>> +.CP0_Config3 = MIPS_CONFIG3 | (1U << CP0C3_M) | (1 << CP0C3_DSP2P) |
>> +   (1 << CP0C3_DSPP),
>>  .CP0_Config4 = MIPS_CONFIG4 | (1U << CP0C4_M),
>>  .CP0_Config4_rw_bitmask = 0,
>>  .CP0_Config5 = MIPS_CONFIG5 | (1 << CP0C5_UFR),
>> @@ -677,7 +679,8 @@ static const mips_def_t mips_defs[] =
>> (2 << CP0C1_DS) | (4 << CP0C1_DL) | (3 << CP0C1_DA) |
>> (1 << CP0C1_PC) | (1 << CP0C1_WR) | (1 << CP0C1_EP),
>>  .CP0_Config2 = MIPS_CONFIG2,
>> -.CP0_Config3 = MIPS_CONFIG3 | (1 << CP0C3_LPA),
>> +.CP0_Config3 = MIPS_CONFIG3 | (1U << CP0C3_M) | (1 << CP0C3_DSP2P) |
>> +   (1 << CP0C3_DSPP) | (1 << CP0C3_LPA),
>>  .CP0_LLAddr_rw_bitmask = 0,
>>  .CP0_LLAddr_shift = 0,
>>  .SYNCI_Step = 32,
>>

When I've been applying this patch to my mips-next candidate branch for
2.2 I realized that you haven't rebased it onto the recent version where
MSA has been added to mips32r5-generic. Now I don't think that having
DSP and MSA on one CPU makes sense, therefore what I'm going to do is to
change mips32r5-generic part in your patch slightly: instead of setting
CP0.Config3.DSP/DSP2P the patch will remove ASE_DSP/DSPR2 insn_flags.

Regards,
Leon

Re: [Qemu-devel] [PATCH] mips: Respect CP0.Status.CU1 for microMIPS FP branches

[Leon Alrae]

On 05/11/2014 20:16, Maciej W. Rozycki wrote:
>  Now as to CP0.Status.CU1, while fixing the 5Kc and 5KEc processors is an 
> obvious change, I think the removal of the extra check may not be such.  
> The thing is in the original architecture -- and it still stands for CP2 
> -- these bits used to control external coprocessors that may or may not 
> have been present.  For example to have an R3000 processor with an FPU you 
> wired an external R3010 unit to it.  Consequently all the CP0.Status.CUx 
> bits were always writable and the relevant logic to intereact with the 
> external chip enabled when requested.
> 
>  And there is software that relies on this property as prior to the 
> introduction of the modern MIPS architecture there was no CP0.Config1 
> register present to check the presence of an FPU with.  Instead what 
> software was supposed to do was to enable CP1, execute a CFC instruction 
> to read CP1.FIR that was supposed not to trap under these circumstances, 
> and check the value obtained in the Imp field (as it was then called) aka 
> ProcessorID.  If that was non-zero, an FPU was present, if it was zero 
> (due to the floating bus being strapped IIUC), no FPU was available. [2] 
> For example we have code in Linux doing this when run on the relevant 
> hardware.
> 
>  Obviously COP1X instructions would trap on CP0.Status.CU3 instead and for 
> example the FP emulator that we have in Linux is prepared for this 
> situation (whether it should really emulate a full MIPS IV if not higher 
> FP instruction set on a lower-ISA processor is another question).
> 
>  And I think all this draws the "right" implementation that we can make, 
> across all the three coprocessors actually, e.g. for CP1, except stores 
> and transfers from:
> 
> check_cp1_enabled(ctx);
> if (ctx->CP0_Config1 & (1 << CP0C1_FP)) {
> handle_the_op();
> }
> 
> for stores and transfers from:
> 
> check_cp1_enabled(ctx);
> if (ctx->CP0_Config1 & (1 << CP0C1_FP)) {
> write_destination(read(source));
> } else {
> write_destination(0);
> }

Yes, if we take into account also legacy CPUs, then this sounds like a
good way to go (instead of just removing extra check).

Thanks,
Leon

Re: [Qemu-devel] [PATCH] mips: Respect CP0.Status.CU1 for microMIPS FP branches

[Maciej W. Rozycki]

On Wed, 5 Nov 2014, Leon Alrae wrote:

> > qemu-umips-cu1-ex.diff
> > Index: qemu-git-trunk/target-mips/translate.c
> > ===
> > --- qemu-git-trunk.orig/target-mips/translate.c 2014-10-27 
> > 04:26:57.0 +
> > +++ qemu-git-trunk/target-mips/translate.c  2014-10-27 04:45:22.838923200 
> > +
> > @@ -13170,8 +13170,13 @@ static void decode_micromips32_opc (CPUM
> >  check_insn(ctx, ASE_MIPS3D);
> >  /* Fall through */
> >  do_cp1branch:
> > -gen_compute_branch1(ctx, mips32_op,
> > -(ctx->opcode >> 18) & 0x7, imm << 1);
> > +if (env->CP0_Config1 & (1 << CP0C1_FP)) {
> 
> I'm wondering if this test is needed at all, I would expect that
> check_cp1_enabled(ctx) is enough. Is it ever possible to have Status.CU1
> set to 1 if FPU isn't present? In translate_init.c the 5Kc CPU (and 5KEc
> you are introducing) is the only CPU without FPU that has Status.CU1
> writeable, which I'm not sure if it's correct. Probably the best way
> would be to check that on the real 5KEc which you seem to have handy :)

 Good point, CP0.Status.CU1 being writable on a non-FPU MIPS architecture 
(non-legacy) processor looks like a bug to me, I missed that detail.  
Here's what the original 5K manual[1] says:

"Coprocessor 1 and 2 can only be marked as usable if a
coprocessor is actually attached to the CPU.  For example,
if no coprocessor 2 is attached, software cannot set CU2.
Note that COP1X instructions are enabled by CU1.

"CU3 is unused by the processor but is implemented as a
read/write bit for backwards-compatibility.  This bit can
only be set to 1 if coprocessor 1 is attached to the CPU."

 I only have a bunch of 5KEf boards I'm afraid.  I could have a look at a 
5Kc sometime, but I don't have the system handy.  It may be easier for you 
to track down a 5K core card within Imagination somewhere.  I think the 
reference above should be enough though, so I'll cook up an update patch 
in the few next days, I think as an incremental one.

> Since this test is used also in other places in existing code and
> potential cleanup won't happen before 2.2 release due to incoming
> hard-freeze, this patch looks good to me.

 That sounds reasonable to me and thanks for the heads-up about the 
impending code freeze.  I think I'll post all the outstanding changes 
regardless to get the reviews rolling as the less obvious stuff may 
require further work and I'd like to get them integrated as soon as 2.3 
opens then.

 Now as to CP0.Status.CU1, while fixing the 5Kc and 5KEc processors is an 
obvious change, I think the removal of the extra check may not be such.  
The thing is in the original architecture -- and it still stands for CP2 
-- these bits used to control external coprocessors that may or may not 
have been present.  For example to have an R3000 processor with an FPU you 
wired an external R3010 unit to it.  Consequently all the CP0.Status.CUx 
bits were always writable and the relevant logic to intereact with the 
external chip enabled when requested.

 And there is software that relies on this property as prior to the 
introduction of the modern MIPS architecture there was no CP0.Config1 
register present to check the presence of an FPU with.  Instead what 
software was supposed to do was to enable CP1, execute a CFC instruction 
to read CP1.FIR that was supposed not to trap under these circumstances, 
and check the value obtained in the Imp field (as it was then called) aka 
ProcessorID.  If that was non-zero, an FPU was present, if it was zero 
(due to the floating bus being strapped IIUC), no FPU was available. [2] 
For example we have code in Linux doing this when run on the relevant 
hardware.

 Obviously COP1X instructions would trap on CP0.Status.CU3 instead and for 
example the FP emulator that we have in Linux is prepared for this 
situation (whether it should really emulate a full MIPS IV if not higher 
FP instruction set on a lower-ISA processor is another question).

 And I think all this draws the "right" implementation that we can make, 
across all the three coprocessors actually, e.g. for CP1, except stores 
and transfers from:

check_cp1_enabled(ctx);
if (ctx->CP0_Config1 & (1 << CP0C1_FP)) {
handle_the_op();
}

for stores and transfers from:

check_cp1_enabled(ctx);
if (ctx->CP0_Config1 & (1 << CP0C1_FP)) {
write_destination(read(source));
} else {
write_destination(0);
}

That is for an absent unit stores and transfers write 0 to their 
destination and all other instructions are treated as NOPs.

 For CP2 that we don't have support for our implementation would be dummy, 
omitting the conditional check along the following "then" part, and for 
CP3 the corresponding enable check would have to take ISA specifics into 
account.

 There's one further complication in that doubleword load/store major 
op

[Qemu-devel] [PATCH] mips: Make `helper_float_cvtw_s' consistent with the remaining helpers

[Maciej W. Rozycki]

Move the call to `update_fcr31' in `helper_float_cvtw_s' after the 
exception flag check, for consistency with the remaining helpers that do 
it last too.

Signed-off-by: Maciej W. Rozycki 
---
 I hope there's no question about this, please apply.

  Maciej

qemu-mips-op-helper-cvtw_s-fcr31.diff
Index: qemu-git-trunk/target-mips/op_helper.c
===
--- qemu-git-trunk.orig/target-mips/op_helper.c 2014-11-02 19:23:41.548963320 
+
+++ qemu-git-trunk/target-mips/op_helper.c  2014-11-02 19:23:55.548607403 
+
@@ -2532,11 +2532,11 @@ uint32_t helper_float_cvtw_s(CPUMIPSStat
 uint32_t wt2;
 
 wt2 = float32_to_int32(fst0, &env->active_fpu.fp_status);
-update_fcr31(env, GETPC());
 if (get_float_exception_flags(&env->active_fpu.fp_status)
 & (float_flag_invalid | float_flag_overflow)) {
 wt2 = FP_TO_INT32_OVERFLOW;
 }
+update_fcr31(env, GETPC());
 return wt2;
 }
 

Re: [Qemu-devel] [PATCH] mips: Add M14K and M14Kc MIPS32r2 microMIPS processors

[Maciej W. Rozycki]

On Wed, 5 Nov 2014, Leon Alrae wrote:

> The actual microMIPS CPU definition is indeed a worthwile addition -
> thanks. It was on my TODO list to upstream such a CPU but I haven't got
> round to it.

 You may still be able to contribute here, by adding microMIPS DSP CPUs.  
Regrettably I wasn't able to track down documentation for the M14KE and 
M14KEc cores or I would have submitted entries for them too; I'd need 
the values of CP0.PRId at the very least, although I presume otherwise 
they're just like the M14K and M14Kc with the CP0.Config3.DSP and 
CP0.Config3.DSP2P bits set (that of course also implies CP0.Status.MX 
being writable).

 Thanks for your review.

  Maciej

Re: [Qemu-devel] [PATCH] mips: Set the CP0.Config3.DSP and CP0.Config3.DSP2P bits

[Leon Alrae]

On 04/11/2014 15:41, Maciej W. Rozycki wrote:
> Set the CP0.Config3.DSP2P bit for the 74kf processor and both that bit 
> and the CP0.Config3.DSP bit for the artificial mips32r5-generic and 
> mips64dspr2 processors.  They have the DSPr2 ASE enabled in `insn_flags' 
> and CPUs that implement that ASE need to have both CP0.Config3.DSP and 
> CP0.Config3.DSP2P set or software won't detect its presence.
> 
> Signed-off-by: Maciej W. Rozycki 
> ---
> qemu-mips-config-dsp.diff
> Index: qemu-git-trunk/target-mips/translate_init.c
> ===
> --- qemu-git-trunk.orig/target-mips/translate_init.c  2014-11-04 
> 03:32:21.408100354 +
> +++ qemu-git-trunk/target-mips/translate_init.c   2014-11-04 
> 03:39:48.458972962 +
> @@ -330,7 +330,8 @@ static const mips_def_t mips_defs[] =
> (0 << CP0C1_DS) | (3 << CP0C1_DL) | (1 << CP0C1_DA) |
> (1 << CP0C1_CA),
>  .CP0_Config2 = MIPS_CONFIG2,
> -.CP0_Config3 = MIPS_CONFIG3 | (0 << CP0C3_VInt) | (1 << CP0C3_DSPP),
> +.CP0_Config3 = MIPS_CONFIG3 | (1 << CP0C3_DSP2P) | (1 << CP0C3_DSPP) 
> |
> +   (0 << CP0C3_VInt),
>  .CP0_LLAddr_rw_bitmask = 0,
>  .CP0_LLAddr_shift = 4,
>  .SYNCI_Step = 32,
> @@ -396,7 +397,8 @@ static const mips_def_t mips_defs[] =
> (0 << CP0C1_DS) | (3 << CP0C1_DL) | (1 << CP0C1_DA) |
> (1 << CP0C1_CA),
>  .CP0_Config2 = MIPS_CONFIG2,
> -.CP0_Config3 = MIPS_CONFIG3 | (1U << CP0C3_M),
> +.CP0_Config3 = MIPS_CONFIG3 | (1U << CP0C3_M) | (1 << CP0C3_DSP2P) |
> +   (1 << CP0C3_DSPP),
>  .CP0_Config4 = MIPS_CONFIG4 | (1U << CP0C4_M),
>  .CP0_Config4_rw_bitmask = 0,
>  .CP0_Config5 = MIPS_CONFIG5 | (1 << CP0C5_UFR),
> @@ -677,7 +679,8 @@ static const mips_def_t mips_defs[] =
> (2 << CP0C1_DS) | (4 << CP0C1_DL) | (3 << CP0C1_DA) |
> (1 << CP0C1_PC) | (1 << CP0C1_WR) | (1 << CP0C1_EP),
>  .CP0_Config2 = MIPS_CONFIG2,
> -.CP0_Config3 = MIPS_CONFIG3 | (1 << CP0C3_LPA),
> +.CP0_Config3 = MIPS_CONFIG3 | (1U << CP0C3_M) | (1 << CP0C3_DSP2P) |
> +   (1 << CP0C3_DSPP) | (1 << CP0C3_LPA),
>  .CP0_LLAddr_rw_bitmask = 0,
>  .CP0_LLAddr_shift = 0,
>  .SYNCI_Step = 32,
> 

Reviewed-by: Leon Alrae 

Re: [Qemu-devel] [PATCH] mips: Respect CP0.Status.CU1 for microMIPS FP branches

[Leon Alrae]

On 03/11/2014 19:08, Maciej W. Rozycki wrote:
> Make microMIPS FP branches respect CP0.Status.CU1 and trap with a 
> Coprocessor Unusable exception if COP1 has been disabled; also trap if 
> no FPU is present at all.
> 
> Standard MIPS FP instruction encodings have a more regular structure and 
> branches are covered with a single umbrella along other instructions.  
> This is not the case with the microMIPS encoding, this case has to be 
> taken care of explicitly here.  Code to do so has been copied from the 
> standard MIPS code handler for OPC_CP1, in `decode_opc'.
> 
> Problems arising from this bug will generally only show up on user 
> context switches in operating systems making use of lazy FP context 
> switches, such as Linux.  It will also more readily trigger if software 
> FPU emulation is used, either implicitly on a non-float CPU, or forced 
> on a hard-float CPU such as with the "nofpu" Linux kernel command line 
> argument.
> 
> The problem may have been easily missed because we have no hard-float 
> microMIPS CPU configuration present; in fact we have no microMIPS CPU 
> configuration of any kind present.
> 
> Signed-off-by: Maciej W. Rozycki 
> ---
> The latter problem is easily fixed though, with a patch I'll be sending 
> right away.  Meanwhile please apply this one.
> 
>   Maciej
> 
> qemu-umips-cu1-ex.diff
> Index: qemu-git-trunk/target-mips/translate.c
> ===
> --- qemu-git-trunk.orig/target-mips/translate.c   2014-10-27 
> 04:26:57.0 +
> +++ qemu-git-trunk/target-mips/translate.c2014-10-27 04:45:22.838923200 
> +
> @@ -13170,8 +13170,13 @@ static void decode_micromips32_opc (CPUM
>  check_insn(ctx, ASE_MIPS3D);
>  /* Fall through */
>  do_cp1branch:
> -gen_compute_branch1(ctx, mips32_op,
> -(ctx->opcode >> 18) & 0x7, imm << 1);
> +if (env->CP0_Config1 & (1 << CP0C1_FP)) {

I'm wondering if this test is needed at all, I would expect that
check_cp1_enabled(ctx) is enough. Is it ever possible to have Status.CU1
set to 1 if FPU isn't present? In translate_init.c the 5Kc CPU (and 5KEc
you are introducing) is the only CPU without FPU that has Status.CU1
writeable, which I'm not sure if it's correct. Probably the best way
would be to check that on the real 5KEc which you seem to have handy :)

Since this test is used also in other places in existing code and
potential cleanup won't happen before 2.2 release due to incoming
hard-freeze, this patch looks good to me.

> +check_cp1_enabled(ctx);
> +gen_compute_branch1(ctx, mips32_op,
> +(ctx->opcode >> 18) & 0x7, imm << 1);
> +} else {
> +generate_exception_err(ctx, EXCP_CpU, 1);
> +}
>  break;
>  case BPOSGE64:
>  case BPOSGE32:
> 

Reviewed-by: Leon Alrae 

Regards,
Leon

Re: [Qemu-devel] [PATCH] mips: Add M14K and M14Kc MIPS32r2 microMIPS processors

[Leon Alrae]

On 04/11/2014 15:39, Maciej W. Rozycki wrote:
> Add the M14K and M14Kc processors from MIPS Technologies that are the 
> original implementation of the microMIPS ISA.  They are dual instruction 
> set processors, implementing both the microMIPS and the standard MIPSr32 
> ISA.
> 
> These processors correspond to the M4K and 4KEc CPUs respectively, 
> except with support for the microMIPS instruction set added, support for 
> the MCU ASE added and two extra interrupt lines, making a total of 8 
> hardware interrupts plus 2 software interrupts.  The remaining parts of 
> the microarchitecture, in particular the pipeline, stayed unchanged.
> 
> The presence of the microMIPS ASE is is reflected in the configuration 
> added.  We currently have no support for the MCU ASE, including in 
> particular the ACLR, ASET and IRET instructions in either encoding, and 
> we have no support for the extra interrupt lines, including bits in 
> CP0.Status and CP0.Cause registers, so these features are not marked, 
> making our support diverge from real hardware.
> 
> Signed-off-by: Sandra Loosemore 
> Signed-off-by: Maciej W. Rozycki 
> ---
>  Hopefully we'll get the missing features sometime sooner rather than 
> later, they should not be difficult to add.  Meanwhile having actual 
> microMIPS CPUs to select is I think a worthwhile addition.  Please 
> apply.
> 
>   Maciej
> 
> qemu-mips-m14k.diff
> Index: qemu-git-trunk/target-mips/translate_init.c
> ===
> --- qemu-git-trunk.orig/target-mips/translate_init.c  2014-11-03 
> 19:09:06.0 +
> +++ qemu-git-trunk/target-mips/translate_init.c   2014-11-04 
> 00:33:42.268947442 +
> @@ -344,6 +344,47 @@ static const mips_def_t mips_defs[] =
>  .mmu_type = MMU_TYPE_R4000,
>  },
>  {
> +.name = "M14K",
> +.CP0_PRid = 0x00019b00,
> +/* Config1 implemented, fixed mapping MMU,
> +   no virtual icache, uncached coherency. */
> +.CP0_Config0 = MIPS_CONFIG0 | (0x2 << CP0C0_KU) | (0x2 << CP0C0_K23) 
> |
> +   (0x1 << CP0C0_AR) | (MMU_TYPE_FMT << CP0C0_MT),
> +.CP0_Config1 = MIPS_CONFIG1,
> +.CP0_Config2 = MIPS_CONFIG2,
> +.CP0_Config3 = MIPS_CONFIG3 | (0x2 << CP0C3_ISA) | (1 << CP0C3_VInt),
> +.CP0_LLAddr_rw_bitmask = 0,
> +.CP0_LLAddr_shift = 4,
> +.SYNCI_Step = 32,
> +.CCRes = 2,
> +.CP0_Status_rw_bitmask = 0x1258FF17,
> +.SEGBITS = 32,
> +.PABITS = 32,
> +.insn_flags = CPU_MIPS32R2 | ASE_MICROMIPS,
> +.mmu_type = MMU_TYPE_FMT,
> +},
> +{
> +.name = "M14Kc",
> +/* This is the TLB-based MMU core.  */
> +.CP0_PRid = 0x00019c00,
> +.CP0_Config0 = MIPS_CONFIG0 | (0x1 << CP0C0_AR) |
> +   (MMU_TYPE_R4000 << CP0C0_MT),
> +.CP0_Config1 = MIPS_CONFIG1 | (15 << CP0C1_MMU) |
> +   (0 << CP0C1_IS) | (3 << CP0C1_IL) | (1 << CP0C1_IA) |
> +   (0 << CP0C1_DS) | (3 << CP0C1_DL) | (1 << CP0C1_DA),
> +.CP0_Config2 = MIPS_CONFIG2,
> +.CP0_Config3 = MIPS_CONFIG3 | (0x2 << CP0C3_ISA) | (0 << CP0C3_VInt),
> +.CP0_LLAddr_rw_bitmask = 0,
> +.CP0_LLAddr_shift = 4,
> +.SYNCI_Step = 32,
> +.CCRes = 2,
> +.CP0_Status_rw_bitmask = 0x1278FF17,
> +.SEGBITS = 32,
> +.PABITS = 32,
> +.insn_flags = CPU_MIPS32R2 | ASE_MICROMIPS,
> +.mmu_type = MMU_TYPE_R4000,
> +},
> +{
>  /* A generic CPU providing MIPS32 Release 5 features.
> FIXME: Eventually this should be replaced by a real CPU model. */
>  .name = "mips32r5-generic",
> 

The actual microMIPS CPU definition is indeed a worthwile addition -
thanks. It was on my TODO list to upstream such a CPU but I haven't got
round to it.

Reviewed-by: Leon Alrae 

Regards,
Leon

Re: [Qemu-devel] [PATCH] mips: Make CP0.Config4 and CP0.Config5 registers signed

[Leon Alrae]

On 04/11/2014 15:37, Maciej W. Rozycki wrote:
> Make the data type used for the CP0.Config4 and CP0.Config5 registers 
> and their mask signed, for consistency with the remaining 32-bit CP0 
> registers, like CP0.Config0, etc.
> 
> Signed-off-by: Maciej W. Rozycki 
> ---
> qemu-mips-config-int32_t.diff
> Index: qemu-git-trunk/target-mips/cpu.h
> ===
> --- qemu-git-trunk.orig/target-mips/cpu.h 2014-11-02 01:05:19.0 
> +
> +++ qemu-git-trunk/target-mips/cpu.h  2014-11-02 01:08:26.527563002 +
> @@ -372,11 +372,11 @@ struct CPUMIPSState {
>  #define CP0C3_MT   2
>  #define CP0C3_SM   1
>  #define CP0C3_TL   0
> -uint32_t CP0_Config4;
> -uint32_t CP0_Config4_rw_bitmask;
> +int32_t CP0_Config4;
> +int32_t CP0_Config4_rw_bitmask;
>  #define CP0C4_M31
> -uint32_t CP0_Config5;
> -uint32_t CP0_Config5_rw_bitmask;
> +int32_t CP0_Config5;
> +int32_t CP0_Config5_rw_bitmask;
>  #define CP0C5_M  31
>  #define CP0C5_K  30
>  #define CP0C5_CV 29
> 

Reviewed-by: Leon Alrae 

Re: [Qemu-devel] [PATCH] mips: Remove CONFIG_VT82C686 from non-Fulong configs

[Leon Alrae]

On 03/11/2014 18:36, Maciej W. Rozycki wrote:
> Fix the regression introduced with commit 
> 47934d0aadc075b05ce2d9e8a44fa6a46edd1afa [hw: move ISA bridges and 
> devices to hw/isa/, configure with default-configs/], by removing 
> CONFIG_VT82C686 from configurations that previously did not enable it.  
> That southbridge is only available on Fulong platforms (CONFIG_FULONG) 
> that are exclusively little-endian, 64-bit MIPS.  Previously vt82c686.o 
> was pulled explicitly with obj-$(CONFIG_FULONG).
> 
> Signed-off-by: Maciej W. Rozycki 
> ---
> Hi,
> 
>  Trivial stuff first, tougher later on.  Compile-tested only, this 
> addresses a regression and should be obviously correct.  Please apply.
> 
>  Thanks,
> 
>   Maciej
> 
> qemu-mipseb-fulong-vt82c686.diff
> Index: qemu-git-trunk/default-configs/mips-softmmu.mak
> ===
> --- qemu-git-trunk.orig/default-configs/mips-softmmu.mak  2014-03-01 
> 02:45:51.0 +
> +++ qemu-git-trunk/default-configs/mips-softmmu.mak   2014-10-28 
> 23:01:51.178971390 +
> @@ -32,6 +32,5 @@ CONFIG_G364FB=y
>  CONFIG_I8259=y
>  CONFIG_JAZZ_LED=y
>  CONFIG_MC146818RTC=y
> -CONFIG_VT82C686=y
>  CONFIG_ISA_TESTDEV=y
>  CONFIG_EMPTY_SLOT=y
> Index: qemu-git-trunk/default-configs/mips64-softmmu.mak
> ===
> --- qemu-git-trunk.orig/default-configs/mips64-softmmu.mak2014-03-01 
> 02:45:51.0 +
> +++ qemu-git-trunk/default-configs/mips64-softmmu.mak 2014-10-28 
> 23:02:09.678926473 +
> @@ -32,6 +32,5 @@ CONFIG_G364FB=y
>  CONFIG_I8259=y
>  CONFIG_JAZZ_LED=y
>  CONFIG_MC146818RTC=y
> -CONFIG_VT82C686=y
>  CONFIG_ISA_TESTDEV=y
>  CONFIG_EMPTY_SLOT=y
> Index: qemu-git-trunk/default-configs/mipsel-softmmu.mak
> ===
> --- qemu-git-trunk.orig/default-configs/mipsel-softmmu.mak2014-03-01 
> 02:45:51.0 +
> +++ qemu-git-trunk/default-configs/mipsel-softmmu.mak 2014-10-28 
> 23:02:26.677701438 +
> @@ -32,6 +32,5 @@ CONFIG_G364FB=y
>  CONFIG_I8259=y
>  CONFIG_JAZZ_LED=y
>  CONFIG_MC146818RTC=y
> -CONFIG_VT82C686=y
>  CONFIG_ISA_TESTDEV=y
>  CONFIG_EMPTY_SLOT=y
> 

Reviewed-by: Leon Alrae 

[Qemu-devel] [PATCH] mips: Set the CP0.Config3.DSP and CP0.Config3.DSP2P bits

[Maciej W. Rozycki]

Set the CP0.Config3.DSP2P bit for the 74kf processor and both that bit 
and the CP0.Config3.DSP bit for the artificial mips32r5-generic and 
mips64dspr2 processors.  They have the DSPr2 ASE enabled in `insn_flags' 
and CPUs that implement that ASE need to have both CP0.Config3.DSP and 
CP0.Config3.DSP2P set or software won't detect its presence.

Signed-off-by: Maciej W. Rozycki 
---
qemu-mips-config-dsp.diff
Index: qemu-git-trunk/target-mips/translate_init.c
===
--- qemu-git-trunk.orig/target-mips/translate_init.c2014-11-04 
03:32:21.408100354 +
+++ qemu-git-trunk/target-mips/translate_init.c 2014-11-04 03:39:48.458972962 
+
@@ -330,7 +330,8 @@ static const mips_def_t mips_defs[] =
(0 << CP0C1_DS) | (3 << CP0C1_DL) | (1 << CP0C1_DA) |
(1 << CP0C1_CA),
 .CP0_Config2 = MIPS_CONFIG2,
-.CP0_Config3 = MIPS_CONFIG3 | (0 << CP0C3_VInt) | (1 << CP0C3_DSPP),
+.CP0_Config3 = MIPS_CONFIG3 | (1 << CP0C3_DSP2P) | (1 << CP0C3_DSPP) |
+   (0 << CP0C3_VInt),
 .CP0_LLAddr_rw_bitmask = 0,
 .CP0_LLAddr_shift = 4,
 .SYNCI_Step = 32,
@@ -396,7 +397,8 @@ static const mips_def_t mips_defs[] =
(0 << CP0C1_DS) | (3 << CP0C1_DL) | (1 << CP0C1_DA) |
(1 << CP0C1_CA),
 .CP0_Config2 = MIPS_CONFIG2,
-.CP0_Config3 = MIPS_CONFIG3 | (1U << CP0C3_M),
+.CP0_Config3 = MIPS_CONFIG3 | (1U << CP0C3_M) | (1 << CP0C3_DSP2P) |
+   (1 << CP0C3_DSPP),
 .CP0_Config4 = MIPS_CONFIG4 | (1U << CP0C4_M),
 .CP0_Config4_rw_bitmask = 0,
 .CP0_Config5 = MIPS_CONFIG5 | (1 << CP0C5_UFR),
@@ -677,7 +679,8 @@ static const mips_def_t mips_defs[] =
(2 << CP0C1_DS) | (4 << CP0C1_DL) | (3 << CP0C1_DA) |
(1 << CP0C1_PC) | (1 << CP0C1_WR) | (1 << CP0C1_EP),
 .CP0_Config2 = MIPS_CONFIG2,
-.CP0_Config3 = MIPS_CONFIG3 | (1 << CP0C3_LPA),
+.CP0_Config3 = MIPS_CONFIG3 | (1U << CP0C3_M) | (1 << CP0C3_DSP2P) |
+   (1 << CP0C3_DSPP) | (1 << CP0C3_LPA),
 .CP0_LLAddr_rw_bitmask = 0,
 .CP0_LLAddr_shift = 0,
 .SYNCI_Step = 32,

[Qemu-devel] [PATCH] mips: Enable vectored interrupt support for the 74Kf CPU

[Maciej W. Rozycki]

Enable vectored interrupt support for the 74Kf CPU, reflecting hardware.

Signed-off-by: Maciej W. Rozycki 
---
qemu-mips-config-74k-vint.diff
Index: qemu-git-trunk/target-mips/translate_init.c
===
--- qemu-git-trunk.orig/target-mips/translate_init.c2014-11-04 
03:39:48.458972962 +
+++ qemu-git-trunk/target-mips/translate_init.c 2014-11-04 03:43:15.479004225 
+
@@ -331,7 +331,7 @@ static const mips_def_t mips_defs[] =
(1 << CP0C1_CA),
 .CP0_Config2 = MIPS_CONFIG2,
 .CP0_Config3 = MIPS_CONFIG3 | (1 << CP0C3_DSP2P) | (1 << CP0C3_DSPP) |
-   (0 << CP0C3_VInt),
+   (1 << CP0C3_VInt),
 .CP0_LLAddr_rw_bitmask = 0,
 .CP0_LLAddr_shift = 4,
 .SYNCI_Step = 32,

[Qemu-devel] [PATCH] mips: Add M14K and M14Kc MIPS32r2 microMIPS processors

[Maciej W. Rozycki]

Add the M14K and M14Kc processors from MIPS Technologies that are the 
original implementation of the microMIPS ISA.  They are dual instruction 
set processors, implementing both the microMIPS and the standard MIPSr32 
ISA.

These processors correspond to the M4K and 4KEc CPUs respectively, 
except with support for the microMIPS instruction set added, support for 
the MCU ASE added and two extra interrupt lines, making a total of 8 
hardware interrupts plus 2 software interrupts.  The remaining parts of 
the microarchitecture, in particular the pipeline, stayed unchanged.

The presence of the microMIPS ASE is is reflected in the configuration 
added.  We currently have no support for the MCU ASE, including in 
particular the ACLR, ASET and IRET instructions in either encoding, and 
we have no support for the extra interrupt lines, including bits in 
CP0.Status and CP0.Cause registers, so these features are not marked, 
making our support diverge from real hardware.

Signed-off-by: Sandra Loosemore 
Signed-off-by: Maciej W. Rozycki 
---
 Hopefully we'll get the missing features sometime sooner rather than 
later, they should not be difficult to add.  Meanwhile having actual 
microMIPS CPUs to select is I think a worthwhile addition.  Please 
apply.

  Maciej

qemu-mips-m14k.diff
Index: qemu-git-trunk/target-mips/translate_init.c
===
--- qemu-git-trunk.orig/target-mips/translate_init.c2014-11-03 
19:09:06.0 +
+++ qemu-git-trunk/target-mips/translate_init.c 2014-11-04 00:33:42.268947442 
+
@@ -344,6 +344,47 @@ static const mips_def_t mips_defs[] =
 .mmu_type = MMU_TYPE_R4000,
 },
 {
+.name = "M14K",
+.CP0_PRid = 0x00019b00,
+/* Config1 implemented, fixed mapping MMU,
+   no virtual icache, uncached coherency. */
+.CP0_Config0 = MIPS_CONFIG0 | (0x2 << CP0C0_KU) | (0x2 << CP0C0_K23) |
+   (0x1 << CP0C0_AR) | (MMU_TYPE_FMT << CP0C0_MT),
+.CP0_Config1 = MIPS_CONFIG1,
+.CP0_Config2 = MIPS_CONFIG2,
+.CP0_Config3 = MIPS_CONFIG3 | (0x2 << CP0C3_ISA) | (1 << CP0C3_VInt),
+.CP0_LLAddr_rw_bitmask = 0,
+.CP0_LLAddr_shift = 4,
+.SYNCI_Step = 32,
+.CCRes = 2,
+.CP0_Status_rw_bitmask = 0x1258FF17,
+.SEGBITS = 32,
+.PABITS = 32,
+.insn_flags = CPU_MIPS32R2 | ASE_MICROMIPS,
+.mmu_type = MMU_TYPE_FMT,
+},
+{
+.name = "M14Kc",
+/* This is the TLB-based MMU core.  */
+.CP0_PRid = 0x00019c00,
+.CP0_Config0 = MIPS_CONFIG0 | (0x1 << CP0C0_AR) |
+   (MMU_TYPE_R4000 << CP0C0_MT),
+.CP0_Config1 = MIPS_CONFIG1 | (15 << CP0C1_MMU) |
+   (0 << CP0C1_IS) | (3 << CP0C1_IL) | (1 << CP0C1_IA) |
+   (0 << CP0C1_DS) | (3 << CP0C1_DL) | (1 << CP0C1_DA),
+.CP0_Config2 = MIPS_CONFIG2,
+.CP0_Config3 = MIPS_CONFIG3 | (0x2 << CP0C3_ISA) | (0 << CP0C3_VInt),
+.CP0_LLAddr_rw_bitmask = 0,
+.CP0_LLAddr_shift = 4,
+.SYNCI_Step = 32,
+.CCRes = 2,
+.CP0_Status_rw_bitmask = 0x1278FF17,
+.SEGBITS = 32,
+.PABITS = 32,
+.insn_flags = CPU_MIPS32R2 | ASE_MICROMIPS,
+.mmu_type = MMU_TYPE_R4000,
+},
+{
 /* A generic CPU providing MIPS32 Release 5 features.
FIXME: Eventually this should be replaced by a real CPU model. */
 .name = "mips32r5-generic",

[Qemu-devel] [PATCH] mips: Make CP0.Config4 and CP0.Config5 registers signed

[Maciej W. Rozycki]

Make the data type used for the CP0.Config4 and CP0.Config5 registers 
and their mask signed, for consistency with the remaining 32-bit CP0 
registers, like CP0.Config0, etc.

Signed-off-by: Maciej W. Rozycki 
---
qemu-mips-config-int32_t.diff
Index: qemu-git-trunk/target-mips/cpu.h
===
--- qemu-git-trunk.orig/target-mips/cpu.h   2014-11-02 01:05:19.0 
+
+++ qemu-git-trunk/target-mips/cpu.h2014-11-02 01:08:26.527563002 +
@@ -372,11 +372,11 @@ struct CPUMIPSState {
 #define CP0C3_MT   2
 #define CP0C3_SM   1
 #define CP0C3_TL   0
-uint32_t CP0_Config4;
-uint32_t CP0_Config4_rw_bitmask;
+int32_t CP0_Config4;
+int32_t CP0_Config4_rw_bitmask;
 #define CP0C4_M31
-uint32_t CP0_Config5;
-uint32_t CP0_Config5_rw_bitmask;
+int32_t CP0_Config5;
+int32_t CP0_Config5_rw_bitmask;
 #define CP0C5_M  31
 #define CP0C5_K  30
 #define CP0C5_CV 29

[Qemu-devel] [PATCH] mips: Add macros for CP0.Config3 and CP0.Config4 bits

[Maciej W. Rozycki]

Define macros for CP0.Config3 and CP0.Config4 bits.  These used to be 
exhaustive as at MIPS32r3, but more bits may have been added since.

Signed-off-by: Maciej W. Rozycki 
---
 More can be added later on.  For the time being, please apply.

  Maciej

qemu-mips-config.diff
Index: qemu-git-trunk/target-mips/cpu.h
===
--- qemu-git-trunk.orig/target-mips/cpu.h   2014-11-02 01:08:26.527563002 
+
+++ qemu-git-trunk/target-mips/cpu.h2014-11-02 01:09:03.528200583 +
@@ -362,19 +362,34 @@ struct CPUMIPSState {
 #define CP0C2_SA   0
 int32_t CP0_Config3;
 #define CP0C3_M31
+#define CP0C3_BPG  30
+#define CP0C3_CMCGR 29
+#define CP0C3_IPLW 21
+#define CP0C3_MMAR 18
+#define CP0C3_MCU  17
 #define CP0C3_ISA_ON_EXC 16
+#define CP0C3_ISA  14
 #define CP0C3_ULRI 13
+#define CP0C3_RXI  12
+#define CP0C3_DSP2P 11
 #define CP0C3_DSPP 10
 #define CP0C3_LPA  7
 #define CP0C3_VEIC 6
 #define CP0C3_VInt 5
 #define CP0C3_SP   4
+#define CP0C3_CDMM 3
 #define CP0C3_MT   2
 #define CP0C3_SM   1
 #define CP0C3_TL   0
 int32_t CP0_Config4;
 int32_t CP0_Config4_rw_bitmask;
 #define CP0C4_M31
+#define CP0C4_KScrExist 16
+#define CP0C4_MMUExtDef 14
+#define CP0C4_FTLBPageSize 8
+#define CP0C4_FTLBWays 4
+#define CP0C4_FTLBSets 0
+#define CP0C4_MMUSizeExt 0
 int32_t CP0_Config5;
 int32_t CP0_Config5_rw_bitmask;
 #define CP0C5_M  31

[Qemu-devel] [PATCH] mips: Add 5KEc and 5KEf MIPS64r2 processors

[Maciej W. Rozycki]

Add the 5KEc and 5KEf processors from MIPS Technologies that are the 
original implementation of the MIPS64r2 ISA.

Silicon for these processors has never been taped out and no soft cores 
were released even.  They do exist though, a CP0.PRId value has been 
assigned and experimental RTLs produced at the time the MIPS64r2 ISA has 
been finalized.  The settings introduced here faithfully reproduce that 
hardware.

As far the implementation goes these processors are the same as the 5Kc 
and the 5Kf CPUs respectively, except implementing the MIPS64r2 rather 
than the original MIPS64 instruction set.  There must have been some 
updates to the CP0 architecture as mandated by the ISA, such as the 
addition of the EBase register, although I am not sure about the exact 
details, no documentation has ever been produced for these processors.  
The remaining parts of the microarchitecture, in particular the 
pipeline, stayed unchanged.  Or to put it another way, the difference 
between a 5K and a 5KE CPU corresponds to one between a 4K and a 4KE 
CPU, except for the 64-bit rather than 32-bit ISA.

Signed-off-by: Maciej W. Rozycki 
---
For the curious:

$ cat /proc/cpuinfo
system type : MIPS Malta
processor   : 0
cpu model   : MIPS 5KE V0.12  FPU V0.12
BogoMIPS: 49.86
wait instruction: no
microsecond timers  : yes
tlb_entries : 32
extra interrupt vector  : yes
hardware watchpoint : yes, count: 2, address/irw mask: [0x0fff, 0x0fff]
microMIPS   : no
ASEs implemented:
shadow register sets: 1
core: 0
VCED exceptions : not available
VCEI exceptions : not available
$ 

-- this is on real hardware, running a 5KEf RTL out of an FPGA.

 Please apply.

  Maciej

qemu-mips-5ke.diff
Index: qemu-git-trunk/target-mips/translate_init.c
===
--- qemu-git-trunk.orig/target-mips/translate_init.c2014-11-02 
18:07:08.0 +
+++ qemu-git-trunk/target-mips/translate_init.c 2014-11-02 18:15:44.108928770 
+
@@ -516,6 +516,51 @@ static const mips_def_t mips_defs[] =
 .mmu_type = MMU_TYPE_R4000,
 },
 {
+.name = "5KEc",
+.CP0_PRid = 0x00018900,
+.CP0_Config0 = MIPS_CONFIG0 | (0x1 << CP0C0_AR) | (0x2 << CP0C0_AT) |
+   (MMU_TYPE_R4000 << CP0C0_MT),
+.CP0_Config1 = MIPS_CONFIG1 | (31 << CP0C1_MMU) |
+   (1 << CP0C1_IS) | (4 << CP0C1_IL) | (1 << CP0C1_IA) |
+   (1 << CP0C1_DS) | (4 << CP0C1_DL) | (1 << CP0C1_DA) |
+   (1 << CP0C1_PC) | (1 << CP0C1_WR) | (1 << CP0C1_EP),
+.CP0_Config2 = MIPS_CONFIG2,
+.CP0_Config3 = MIPS_CONFIG3,
+.CP0_LLAddr_rw_bitmask = 0,
+.CP0_LLAddr_shift = 4,
+.SYNCI_Step = 32,
+.CCRes = 2,
+.CP0_Status_rw_bitmask = 0x32F8,
+.SEGBITS = 42,
+.PABITS = 36,
+.insn_flags = CPU_MIPS64R2,
+.mmu_type = MMU_TYPE_R4000,
+},
+{
+.name = "5KEf",
+.CP0_PRid = 0x00018900,
+.CP0_Config0 = MIPS_CONFIG0 | (0x1 << CP0C0_AR) | (0x2 << CP0C0_AT) |
+   (MMU_TYPE_R4000 << CP0C0_MT),
+.CP0_Config1 = MIPS_CONFIG1 | (1 << CP0C1_FP) | (31 << CP0C1_MMU) |
+   (1 << CP0C1_IS) | (4 << CP0C1_IL) | (1 << CP0C1_IA) |
+   (1 << CP0C1_DS) | (4 << CP0C1_DL) | (1 << CP0C1_DA) |
+   (1 << CP0C1_PC) | (1 << CP0C1_WR) | (1 << CP0C1_EP),
+.CP0_Config2 = MIPS_CONFIG2,
+.CP0_Config3 = MIPS_CONFIG3,
+.CP0_LLAddr_rw_bitmask = 0,
+.CP0_LLAddr_shift = 4,
+.SYNCI_Step = 32,
+.CCRes = 2,
+.CP0_Status_rw_bitmask = 0x36F8,
+.CP1_fcr0 = (1 << FCR0_F64) | (1 << FCR0_L) | (1 << FCR0_W) |
+(1 << FCR0_D) | (1 << FCR0_S) |
+(0x89 << FCR0_PRID) | (0x0 << FCR0_REV),
+.SEGBITS = 42,
+.PABITS = 36,
+.insn_flags = CPU_MIPS64R2,
+.mmu_type = MMU_TYPE_R4000,
+},
+{
 /* A generic CPU supporting MIPS64 Release 6 ISA.
FIXME: It does not support all the MIPS64R6 features yet.
   Eventually this should be replaced by a real CPU model. */

[Qemu-devel] [PATCH] mips: Respect CP0.Status.CU1 for microMIPS FP branches

[Maciej W. Rozycki]

Make microMIPS FP branches respect CP0.Status.CU1 and trap with a 
Coprocessor Unusable exception if COP1 has been disabled; also trap if 
no FPU is present at all.

Standard MIPS FP instruction encodings have a more regular structure and 
branches are covered with a single umbrella along other instructions.  
This is not the case with the microMIPS encoding, this case has to be 
taken care of explicitly here.  Code to do so has been copied from the 
standard MIPS code handler for OPC_CP1, in `decode_opc'.

Problems arising from this bug will generally only show up on user 
context switches in operating systems making use of lazy FP context 
switches, such as Linux.  It will also more readily trigger if software 
FPU emulation is used, either implicitly on a non-float CPU, or forced 
on a hard-float CPU such as with the "nofpu" Linux kernel command line 
argument.

The problem may have been easily missed because we have no hard-float 
microMIPS CPU configuration present; in fact we have no microMIPS CPU 
configuration of any kind present.

Signed-off-by: Maciej W. Rozycki 
---
The latter problem is easily fixed though, with a patch I'll be sending 
right away.  Meanwhile please apply this one.

  Maciej

qemu-umips-cu1-ex.diff
Index: qemu-git-trunk/target-mips/translate.c
===
--- qemu-git-trunk.orig/target-mips/translate.c 2014-10-27 04:26:57.0 
+
+++ qemu-git-trunk/target-mips/translate.c  2014-10-27 04:45:22.838923200 
+
@@ -13170,8 +13170,13 @@ static void decode_micromips32_opc (CPUM
 check_insn(ctx, ASE_MIPS3D);
 /* Fall through */
 do_cp1branch:
-gen_compute_branch1(ctx, mips32_op,
-(ctx->opcode >> 18) & 0x7, imm << 1);
+if (env->CP0_Config1 & (1 << CP0C1_FP)) {
+check_cp1_enabled(ctx);
+gen_compute_branch1(ctx, mips32_op,
+(ctx->opcode >> 18) & 0x7, imm << 1);
+} else {
+generate_exception_err(ctx, EXCP_CpU, 1);
+}
 break;
 case BPOSGE64:
 case BPOSGE32:

[Qemu-devel] [PATCH] mips/gdbstub: Make CP1.FIR read-only here too

[Maciej W. Rozycki]

CP1.FIR is read-only in hardware so gdbstub must respect it.  We already 
respect it for CTC1 instructions, so do it here too.

Signed-off-by: Maciej W. Rozycki 
---
Not much to say about it here.  Please apply.

  Maciej

qemu-mips-fir.diff
Index: qemu-git-trunk/target-mips/gdbstub.c
===
--- qemu-git-trunk.orig/target-mips/gdbstub.c   2014-11-02 17:51:33.458968203 
+
+++ qemu-git-trunk/target-mips/gdbstub.c2014-11-02 17:51:35.958924223 
+
@@ -112,7 +112,7 @@ int mips_cpu_gdb_write_register(CPUState
 RESTORE_ROUNDING_MODE;
 break;
 case 71:
-env->active_fpu.fcr0 = tmp;
+/* FIR is read-only.  Ignore writes.  */
 break;
 }
 return sizeof(target_ulong);

[Qemu-devel] [PATCH] mips/gdbstub: Correct the handling of register #72 on writes

[Maciej W. Rozycki]

Fix an off-by-one error in `mips_cpu_gdb_write_register' for register 
#72 that is handled further down in that function rather than here, 
matching how `mips_cpu_gdb_read_register' handles it.  This register 
slot is a fake anyway, there's nothing in hardware that corresponds to 
it.

Signed-off-by: Maciej W. Rozycki 
---
 I have a further change down the queue to clean up 
`mips_cpu_gdb_read_register' and `mips_cpu_gdb_write_register' and make 
them more consistent with respect to each other as far as the handling 
of FP registers is concerned.  For now please apply this obvious change.  
Thanks.

  Maciej

qemu-mips-fpreg72.diff
Index: qemu-git-trunk/target-mips/gdbstub.c
===
--- qemu-git-trunk.orig/target-mips/gdbstub.c   2013-07-29 11:23:07.048742983 
+0100
+++ qemu-git-trunk/target-mips/gdbstub.c2014-10-27 04:17:19.159003270 
+
@@ -97,7 +97,7 @@ int mips_cpu_gdb_write_register(CPUState
 return sizeof(target_ulong);
 }
 if (env->CP0_Config1 & (1 << CP0C1_FP)
-&& n >= 38 && n < 73) {
+&& n >= 38 && n < 72) {
 if (n < 70) {
 if (env->CP0_Status & (1 << CP0St_FR)) {
 env->active_fpu.fpr[n - 38].d = tmp;

[Qemu-devel] [PATCH] mips: Remove CONFIG_VT82C686 from non-Fulong configs

[Maciej W. Rozycki]

Fix the regression introduced with commit 
47934d0aadc075b05ce2d9e8a44fa6a46edd1afa [hw: move ISA bridges and 
devices to hw/isa/, configure with default-configs/], by removing 
CONFIG_VT82C686 from configurations that previously did not enable it.  
That southbridge is only available on Fulong platforms (CONFIG_FULONG) 
that are exclusively little-endian, 64-bit MIPS.  Previously vt82c686.o 
was pulled explicitly with obj-$(CONFIG_FULONG).

Signed-off-by: Maciej W. Rozycki 
---
Hi,

 Trivial stuff first, tougher later on.  Compile-tested only, this 
addresses a regression and should be obviously correct.  Please apply.

 Thanks,

  Maciej

qemu-mipseb-fulong-vt82c686.diff
Index: qemu-git-trunk/default-configs/mips-softmmu.mak
===
--- qemu-git-trunk.orig/default-configs/mips-softmmu.mak2014-03-01 
02:45:51.0 +
+++ qemu-git-trunk/default-configs/mips-softmmu.mak 2014-10-28 
23:01:51.178971390 +
@@ -32,6 +32,5 @@ CONFIG_G364FB=y
 CONFIG_I8259=y
 CONFIG_JAZZ_LED=y
 CONFIG_MC146818RTC=y
-CONFIG_VT82C686=y
 CONFIG_ISA_TESTDEV=y
 CONFIG_EMPTY_SLOT=y
Index: qemu-git-trunk/default-configs/mips64-softmmu.mak
===
--- qemu-git-trunk.orig/default-configs/mips64-softmmu.mak  2014-03-01 
02:45:51.0 +
+++ qemu-git-trunk/default-configs/mips64-softmmu.mak   2014-10-28 
23:02:09.678926473 +
@@ -32,6 +32,5 @@ CONFIG_G364FB=y
 CONFIG_I8259=y
 CONFIG_JAZZ_LED=y
 CONFIG_MC146818RTC=y
-CONFIG_VT82C686=y
 CONFIG_ISA_TESTDEV=y
 CONFIG_EMPTY_SLOT=y
Index: qemu-git-trunk/default-configs/mipsel-softmmu.mak
===
--- qemu-git-trunk.orig/default-configs/mipsel-softmmu.mak  2014-03-01 
02:45:51.0 +
+++ qemu-git-trunk/default-configs/mipsel-softmmu.mak   2014-10-28 
23:02:26.677701438 +
@@ -32,6 +32,5 @@ CONFIG_G364FB=y
 CONFIG_I8259=y
 CONFIG_JAZZ_LED=y
 CONFIG_MC146818RTC=y
-CONFIG_VT82C686=y
 CONFIG_ISA_TESTDEV=y
 CONFIG_EMPTY_SLOT=y

[Qemu-devel] [PATCH] mips jazz: do not raise data bus exception when accessing invalid addresses

[Hervé Poussineau]

MIPS Jazz chipset doesn't seem to raise data bus exceptions on invalid accesses.
However, there is no easy way to prevent them. Creating a big memory region
for the whole address space doesn't prevent memory core to directly call
unassigned_mem_read/write which in turn call cpu->do_unassigned_access,
which (for MIPS CPU) raise an data bus exception.

This fixes a MIPS Jazz regression introduced in 
c658b94f6e8c206c59d02aa6fbac285b86b53d2c.

Signed-off-by: Hervé Poussineau 
---
This fixes a known regression in QEMU 1.6. Let it be fixed as soon as possible.

 hw/mips/mips_jazz.c |   24 
 1 file changed, 24 insertions(+)

diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
index 49bdd02..5f6dd9f 100644
--- a/hw/mips/mips_jazz.c
+++ b/hw/mips/mips_jazz.c
@@ -108,6 +108,18 @@ static void cpu_request_exit(void *opaque, int irq, int 
level)
 }
 }
 
+static CPUUnassignedAccess real_do_unassigned_access;
+static void mips_jazz_do_unassigned_access(CPUState *cpu, hwaddr addr,
+   bool is_write, bool is_exec,
+   int opaque, unsigned size)
+{
+if (!is_exec) {
+/* ignore invalid access (ie do not raise exception) */
+return;
+}
+(*real_do_unassigned_access)(cpu, addr, is_write, is_exec, opaque, size);
+}
+
 static void mips_jazz_init(MemoryRegion *address_space,
MemoryRegion *address_space_io,
ram_addr_t ram_size,
@@ -117,6 +129,7 @@ static void mips_jazz_init(MemoryRegion *address_space,
 char *filename;
 int bios_size, n;
 MIPSCPU *cpu;
+CPUClass *cc;
 CPUMIPSState *env;
 qemu_irq *rc4030, *i8259;
 rc4030_dma *dmas;
@@ -154,6 +167,17 @@ static void mips_jazz_init(MemoryRegion *address_space,
 env = &cpu->env;
 qemu_register_reset(main_cpu_reset, cpu);
 
+/* Chipset returns 0 in invalid reads and do not raise data exceptions.
+ * However, we can't simply add a global memory region to catch
+ * everything, as memory core directly call unassigned_mem_read/write
+ * on some invalid accesses, which call do_unassigned_access on the
+ * CPU, which raise an exception.
+ * Handle that case by hijacking the do_unassigned_access method on
+ * the CPU, and do not raise exceptions for data access. */
+cc = CPU_GET_CLASS(cpu);
+real_do_unassigned_access = cc->do_unassigned_access;
+cc->do_unassigned_access = mips_jazz_do_unassigned_access;
+
 /* allocate RAM */
 memory_region_init_ram(ram, NULL, "mips_jazz.ram", ram_size);
 vmstate_register_ram_global(ram);
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH] mips/malta: prevent writes to reset flash mapping faulting

[James Hogan]

Hi Andreas,

On 23/08/13 12:08, Andreas Färber wrote:
> Am 23.08.2013 09:59, schrieb Leon Alrae:
>> From: James Hogan 
>>
>> Commit a427338 (mips_malta: correct reading MIPS revision at 0x1fc00010)
>> altered the behaviour of the monitor flash mapping at the reset address
>> by making it read only. However this causes data bus error exceptions
>> when it is written to since it is effectively unassigned memory for
>> writes. This isn't how the real hardware behaves. That memory can be
>> written to (even with the MFWR jumper not fitted) and the new value read
>> back from, but it doesn't get written back to the monitor flash so is
>> volatile.
>>
>> This is fixed by converting the bios copy from read only ram to a bios
>> device with a nop write callback.
> 
> That sounds like a contradiction: The nop write will not have reads
> return the new value, will it?

correct.

> Why not just remove the _set_readonly and have it reloaded on reset for
> volatility?

That's what I tried first, but the bios copy is normal ram so it doesn't
get reloaded on reset. I'll have a play to see if I can use rom_add_blob
(although I seem to remember already trying that...).

> Anyway, having a MemoryRegionOps with just a .write looks dangerous, but
> I guess you've tested read to work. We had been seeing assertions
> elsewhere when either was missing.

Yeh reads seem to work fine (it also executes from it fine).

Thanks for taking a look

James

Re: [Qemu-devel] [PATCH] mips/malta: prevent writes to reset flash mapping faulting

[Andreas Färber]

Am 23.08.2013 09:59, schrieb Leon Alrae:
> From: James Hogan 
> 
> Commit a427338 (mips_malta: correct reading MIPS revision at 0x1fc00010)
> altered the behaviour of the monitor flash mapping at the reset address
> by making it read only. However this causes data bus error exceptions
> when it is written to since it is effectively unassigned memory for
> writes. This isn't how the real hardware behaves. That memory can be
> written to (even with the MFWR jumper not fitted) and the new value read
> back from, but it doesn't get written back to the monitor flash so is
> volatile.
> 
> This is fixed by converting the bios copy from read only ram to a bios
> device with a nop write callback.

That sounds like a contradiction: The nop write will not have reads
return the new value, will it?

Why not just remove the _set_readonly and have it reloaded on reset for
volatility?

Anyway, having a MemoryRegionOps with just a .write looks dangerous, but
I guess you've tested read to work. We had been seeing assertions
elsewhere when either was missing.

Regards,
Andreas

> 
> Signed-off-by: James Hogan 
> Cc: Paul Burton 
> Cc: Leon Alrae 
> Cc: Aurelien Jarno 
> Signed-off-by: Leon Alrae 
> ---
>  hw/mips/mips_malta.c |   14 --
>  1 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
> index f8d064c..9e721d3 100644
> --- a/hw/mips/mips_malta.c
> +++ b/hw/mips/mips_malta.c
> @@ -873,6 +873,16 @@ static void cpu_request_exit(void *opaque, int irq, int 
> level)
>  }
>  }
>  
> +static void monflash_copy_mem_write(void *opaque, hwaddr ram_addr,
> +uint64_t val, unsigned size)
> +{
> +}
> +
> +static const MemoryRegionOps monflash_copy_mem_ops = {
> +.write = monflash_copy_mem_write,
> +.endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
>  static
>  void mips_malta_init(QEMUMachineInitArgs *args)
>  {
> @@ -1043,13 +1053,13 @@ void mips_malta_init(QEMUMachineInitArgs *args)
>   * handled by an overlapping region as the resulting ROM code subpage
>   * regions are not executable.
>   */
> -memory_region_init_ram(bios_copy, NULL, "bios.1fc", BIOS_SIZE);
> +memory_region_init_rom_device(bios_copy, NULL, &monflash_copy_mem_ops, 
> NULL,
> +  "bios.1fc", BIOS_SIZE);
>  if (!rom_copy(memory_region_get_ram_ptr(bios_copy),
>FLASH_ADDRESS, BIOS_SIZE)) {
>  memcpy(memory_region_get_ram_ptr(bios_copy),
> memory_region_get_ram_ptr(bios), BIOS_SIZE);
>  }
> -memory_region_set_readonly(bios_copy, true);
>  memory_region_add_subregion(system_memory, RESET_ADDRESS, bios_copy);
>  
>  /* Board ID = 0x420 (Malta Board with CoreLV) */
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH] mips/malta: prevent writes to reset flash mapping faulting

[Leon Alrae]

From: James Hogan 

Commit a427338 (mips_malta: correct reading MIPS revision at 0x1fc00010)
altered the behaviour of the monitor flash mapping at the reset address
by making it read only. However this causes data bus error exceptions
when it is written to since it is effectively unassigned memory for
writes. This isn't how the real hardware behaves. That memory can be
written to (even with the MFWR jumper not fitted) and the new value read
back from, but it doesn't get written back to the monitor flash so is
volatile.

This is fixed by converting the bios copy from read only ram to a bios
device with a nop write callback.

Signed-off-by: James Hogan 
Cc: Paul Burton 
Cc: Leon Alrae 
Cc: Aurelien Jarno 
Signed-off-by: Leon Alrae 
---
 hw/mips/mips_malta.c |   14 --
 1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index f8d064c..9e721d3 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -873,6 +873,16 @@ static void cpu_request_exit(void *opaque, int irq, int 
level)
 }
 }
 
+static void monflash_copy_mem_write(void *opaque, hwaddr ram_addr,
+uint64_t val, unsigned size)
+{
+}
+
+static const MemoryRegionOps monflash_copy_mem_ops = {
+.write = monflash_copy_mem_write,
+.endianness = DEVICE_NATIVE_ENDIAN,
+};
+
 static
 void mips_malta_init(QEMUMachineInitArgs *args)
 {
@@ -1043,13 +1053,13 @@ void mips_malta_init(QEMUMachineInitArgs *args)
  * handled by an overlapping region as the resulting ROM code subpage
  * regions are not executable.
  */
-memory_region_init_ram(bios_copy, NULL, "bios.1fc", BIOS_SIZE);
+memory_region_init_rom_device(bios_copy, NULL, &monflash_copy_mem_ops, 
NULL,
+  "bios.1fc", BIOS_SIZE);
 if (!rom_copy(memory_region_get_ram_ptr(bios_copy),
   FLASH_ADDRESS, BIOS_SIZE)) {
 memcpy(memory_region_get_ram_ptr(bios_copy),
memory_region_get_ram_ptr(bios), BIOS_SIZE);
 }
-memory_region_set_readonly(bios_copy, true);
 memory_region_add_subregion(system_memory, RESET_ADDRESS, bios_copy);
 
 /* Board ID = 0x420 (Malta Board with CoreLV) */
-- 
1.7.5.4

Re: [Qemu-devel] [PATCH] MIPS: Translate breaks and traps into the appropriate signal

[Meador Inge]

On 01/10/2013 04:12 PM, Peter Maydell wrote:

> This is an OS/ABI specific define, right? I don't think it
> belongs in the target-mips header file. Since it only has one
> user, I think you could reasonably just put it in linux-user/main.c.

The enum will only be used in the MIPS CPU loop.  I originally put it in
target-mips/cpu.h because that is where the exception codes
are defined.   However, the one user argument makes sense to me.  I
moved the enum definition.

Thanks for the review.

-- 
Meador Inge
CodeSourcery / Mentor Embedded

Re: [Qemu-devel] [PATCH] MIPS: Translate breaks and traps into the appropriate signal

[Meador Inge]

On 01/10/2013 03:57 PM, Stefan Weil wrote:

> please check your patch before submitting it to qemu-devel.
> See also http://wiki.qemu.org/Contribute/SubmitAPatch.

Ah, thanks for the pointer.  I have fixed the style violations.


-- 
Meador Inge
CodeSourcery / Mentor Embedded

Re: [Qemu-devel] [PATCH] MIPS: Translate breaks and traps into the appropriate signal

[Peter Maydell]

On 10 January 2013 21:46, Meador Inge  wrote:
> --- a/target-mips/cpu.h
> +++ b/target-mips/cpu.h
> @@ -620,6 +620,12 @@ enum {
>  /* Dummy exception for conditional stores.  */
>  #define EXCP_SC 0x100
>
> +/* Break codes */
> +enum {
> +BRK_OVERFLOW = 6,
> +BRK_DIVZERO = 7
> +};

This is an OS/ABI specific define, right? I don't think it
belongs in the target-mips header file. Since it only has one
user, I think you could reasonably just put it in linux-user/main.c.

-- PMM

Re: [Qemu-devel] [PATCH] MIPS: Translate breaks and traps into the appropriate signal

[Stefan Weil]

Am 10.01.2013 22:46, schrieb Meador Inge:

GCC and GAS are capable of generating traps or breaks to check for
division by zero.  Additionally, GAS is capable of generating traps
or breaks to check for overflow on certain division and multiplication
operations.  The Linux kernel translates these traps and breaks into
signals.  This patch implements the corresponding feature in QEMU.

Signed-off-by: Meador Inge 
---
  linux-user/main.c |   64 -
  target-mips/cpu.h |6 +
  2 files changed, 69 insertions(+), 1 deletion(-)





Hi,

please check your patch before submitting it to qemu-devel.
See also http://wiki.qemu.org/Contribute/SubmitAPatch.

Regards,

Stefan W.


$ scripts/checkpatch.pl 
0001-MIPS-Translate-breaks-and-traps-into-the-appropriate.patch

WARNING: braces {} are necessary for all arms of this statement
#62: FILE: linux-user/main.c:2329:
+if (ret != 0)
[...]

WARNING: braces {} are necessary for all arms of this statement
#70: FILE: linux-user/main.c:2337:
+if (code >= (1 << 10))
[...]

WARNING: braces {} are necessary for all arms of this statement
#73: FILE: linux-user/main.c:2340:
+if (do_break(env, &info, code) != 0)
[...]

WARNING: braces {} are necessary for all arms of this statement
#83: FILE: linux-user/main.c:2350:
+if (ret != 0)
[...]

WARNING: braces {} are necessary for all arms of this statement
#87: FILE: linux-user/main.c:2354:
+if (!(trap_instr & 0xFC00))
[...]

WARNING: braces {} are necessary for all arms of this statement
#90: FILE: linux-user/main.c:2357:
+if (do_break(env, &info, code) != 0)
[...]

total: 0 errors, 6 warnings, 89 lines checked

0001-MIPS-Translate-breaks-and-traps-into-the-appropriate.patch has 
style problems, please review.  If any of these errors

are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

[Qemu-devel] [PATCH] MIPS: Translate breaks and traps into the appropriate signal

[Meador Inge]

GCC and GAS are capable of generating traps or breaks to check for
division by zero.  Additionally, GAS is capable of generating traps
or breaks to check for overflow on certain division and multiplication
operations.  The Linux kernel translates these traps and breaks into
signals.  This patch implements the corresponding feature in QEMU.

Signed-off-by: Meador Inge 
---
 linux-user/main.c |   64 -
 target-mips/cpu.h |6 +
 2 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index 9ade1bf..b9532e0 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -2182,6 +2182,26 @@ static int do_store_exclusive(CPUMIPSState *env)
 return segv;
 }
 
+static int do_break(CPUMIPSState *env, target_siginfo_t *info,
+unsigned int code)
+{
+int ret = -1;
+
+switch (code) {
+case BRK_OVERFLOW:
+case BRK_DIVZERO:
+info->si_signo = TARGET_SIGFPE;
+info->si_errno = 0;
+info->si_code = (code == BRK_OVERFLOW) ? FPE_INTOVF : FPE_INTDIV;
+queue_signal(env, info->si_signo, &*info);
+ret = 0;
+default:
+break;
+}
+
+return ret;
+}
+
 void cpu_loop(CPUMIPSState *env)
 {
 target_siginfo_t info;
@@ -2297,8 +2317,50 @@ done_syscall:
 info.si_code = TARGET_ILL_ILLOPC;
 queue_signal(env, info.si_signo, &info);
 break;
+/* The code below was inspired by the MIPS Linux kernel trap
+ * handling code in arch/mips/kernel/traps.c.
+ */
+case EXCP_BREAK:
+{
+abi_ulong trap_instr;
+unsigned int code;
+
+ret = get_user_ual(trap_instr, env->active_tc.PC);
+if (ret != 0)
+goto error;
+
+/* As described in the original Linux kernel code, the
+ * below checks on 'code' are to work around an old
+ * assembly bug.
+ */
+code = ((trap_instr >> 6) & ((1 << 20) - 1));
+if (code >= (1 << 10))
+code >>= 10;
+
+if (do_break(env, &info, code) != 0)
+goto error;
+break;
+}
+case EXCP_TRAP:
+{
+abi_ulong trap_instr;
+unsigned int code = 0;
+
+ret = get_user_ual(trap_instr, env->active_tc.PC);
+if (ret != 0)
+goto error;
+
+/* The immediate versions don't provide a code.  */
+if (!(trap_instr & 0xFC00))
+code = ((trap_instr >> 6) & ((1 << 10) - 1));
+
+if (do_break(env, &info, code) != 0)
+goto error;
+break;
+}
+break;
 default:
-//error:
+error:
 fprintf(stderr, "qemu: unhandled CPU exception 0x%x - aborting\n",
 trapnr);
 cpu_dump_state(env, stderr, fprintf, 0);
diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index 5963d62..c5fbe04 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -620,6 +620,12 @@ enum {
 /* Dummy exception for conditional stores.  */
 #define EXCP_SC 0x100
 
+/* Break codes */
+enum {
+BRK_OVERFLOW = 6,
+BRK_DIVZERO = 7
+};
+
 /*
  * This is an interrnally generated WAKE request line.
  * It is driven by the CPU itself. Raised when the MT
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH] mips/malta: fix CBUS UART interrupt pin

[Aurelien Jarno]

On Wed, Nov 14, 2012 at 07:45:02PM +, Johnson, Eric wrote:
> > -Original Message-
> > From: qemu-devel-bounces+ericj=mips@nongnu.org [mailto:qemu-devel-
> > bounces+ericj=mips@nongnu.org] On Behalf Of Aurelien Jarno
> > Sent: Wednesday, November 14, 2012 6:38 AM
> > To: qemu-devel@nongnu.org
> > Cc: Aurelien Jarno
> > Subject: [Qemu-devel] [PATCH] mips/malta: fix CBUS UART interrupt pin
> > 
> > According to the MIPS Malta Developement Platform User's Manual, the
> > i8259 interrupt controller is supposed to be connected to the hardware
> > IRQ0, and the CBUS UART to the hardware interrupt 2.
> > 
> > In QEMU they are both connected to hardware interrupt 0, the CBUS UART
> > interrupt being wrong. This patch fixes that. It should be noted that
> > the irq array in QEMU includes the software interrupts, hence
> > env->irq[2] is the first hardware interrupt.
> > 
> > Cc: Ralf Baechle 
> > Signed-off-by: Aurelien Jarno 
> > ---
> >  hw/mips_malta.c |3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/mips_malta.c b/hw/mips_malta.c
> > index 0571d58..4d2464a 100644
> > --- a/hw/mips_malta.c
> > +++ b/hw/mips_malta.c
> > @@ -861,7 +861,8 @@ void mips_malta_init(QEMUMachineInitArgs *args)
> >  be = 0;
> >  #endif
> >  /* FPGA */
> > -malta_fpga_init(system_memory, FPGA_ADDRESS, env->irq[2],
> > serial_hds[2]);
> > +/* The CBUS UART is attached to the MIPS CPU INT2 pin, ie interrupt 4
> > */
> > +malta_fpga_init(system_memory, FPGA_ADDRESS, env->irq[4],
> > serial_hds[2]);
> > 
> >  /* Load firmware in flash / BIOS. */
> >  dinfo = drive_get(IF_PFLASH, 0, fl_idx);
> > --
> > 1.7.10.4
> > 
> 
> I double checked with a Malta expert here.  He verified that the CBUS UART is 
> connected to the HW2 interrupt pin.
> 
> Reviewed-by: Eric Johnson 
> 

Thanks for the review, I have applied the patch.


-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net

Re: [Qemu-devel] [PATCH] mips/malta: fix CBUS UART interrupt pin

[Johnson, Eric]

> -Original Message-
> From: qemu-devel-bounces+ericj=mips@nongnu.org [mailto:qemu-devel-
> bounces+ericj=mips@nongnu.org] On Behalf Of Aurelien Jarno
> Sent: Wednesday, November 14, 2012 6:38 AM
> To: qemu-devel@nongnu.org
> Cc: Aurelien Jarno
> Subject: [Qemu-devel] [PATCH] mips/malta: fix CBUS UART interrupt pin
> 
> According to the MIPS Malta Developement Platform User's Manual, the
> i8259 interrupt controller is supposed to be connected to the hardware
> IRQ0, and the CBUS UART to the hardware interrupt 2.
> 
> In QEMU they are both connected to hardware interrupt 0, the CBUS UART
> interrupt being wrong. This patch fixes that. It should be noted that
> the irq array in QEMU includes the software interrupts, hence
> env->irq[2] is the first hardware interrupt.
> 
> Cc: Ralf Baechle 
> Signed-off-by: Aurelien Jarno 
> ---
>  hw/mips_malta.c |3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/mips_malta.c b/hw/mips_malta.c
> index 0571d58..4d2464a 100644
> --- a/hw/mips_malta.c
> +++ b/hw/mips_malta.c
> @@ -861,7 +861,8 @@ void mips_malta_init(QEMUMachineInitArgs *args)
>  be = 0;
>  #endif
>  /* FPGA */
> -malta_fpga_init(system_memory, FPGA_ADDRESS, env->irq[2],
> serial_hds[2]);
> +/* The CBUS UART is attached to the MIPS CPU INT2 pin, ie interrupt 4
> */
> +malta_fpga_init(system_memory, FPGA_ADDRESS, env->irq[4],
> serial_hds[2]);
> 
>  /* Load firmware in flash / BIOS. */
>  dinfo = drive_get(IF_PFLASH, 0, fl_idx);
> --
> 1.7.10.4
> 

I double checked with a Malta expert here.  He verified that the CBUS UART is 
connected to the HW2 interrupt pin.

Reviewed-by: Eric Johnson 

-Eric

[Qemu-devel] [PATCH] mips/malta: fix CBUS UART interrupt pin

[Aurelien Jarno]

According to the MIPS Malta Developement Platform User's Manual, the
i8259 interrupt controller is supposed to be connected to the hardware
IRQ0, and the CBUS UART to the hardware interrupt 2.

In QEMU they are both connected to hardware interrupt 0, the CBUS UART
interrupt being wrong. This patch fixes that. It should be noted that
the irq array in QEMU includes the software interrupts, hence
env->irq[2] is the first hardware interrupt.

Cc: Ralf Baechle 
Signed-off-by: Aurelien Jarno 
---
 hw/mips_malta.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/mips_malta.c b/hw/mips_malta.c
index 0571d58..4d2464a 100644
--- a/hw/mips_malta.c
+++ b/hw/mips_malta.c
@@ -861,7 +861,8 @@ void mips_malta_init(QEMUMachineInitArgs *args)
 be = 0;
 #endif
 /* FPGA */
-malta_fpga_init(system_memory, FPGA_ADDRESS, env->irq[2], serial_hds[2]);
+/* The CBUS UART is attached to the MIPS CPU INT2 pin, ie interrupt 4 */
+malta_fpga_init(system_memory, FPGA_ADDRESS, env->irq[4], serial_hds[2]);
 
 /* Load firmware in flash / BIOS. */
 dinfo = drive_get(IF_PFLASH, 0, fl_idx);
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH] MIPS/user: Fix reset CPU state initialization

[Aurelien Jarno]

On Fri, Jun 08, 2012 at 02:04:40AM +0100, Maciej W. Rozycki wrote:
> 
>  This change updates the CPU reset sequence to use a common piece of code 
> that figures out CPU state flags, fixing the problem with MIPS_HFLAG_COP1X 
> not being set where applicable that causes floating-point MADD family 
> instructions (and other instructions from the MIPS IV FP subset) to trap.
> 
>  As compute_hflags is now shared between op_helper.c and translate.c, the 
> function is now moved to a common header.  There are no changes to this 
> function.
> 
>  The problem was seen with the 24Kf MIPS32r2 processor in user emulation.  
> The new approach prevents system and user emulation from diverging -- all 
> the hflags state is initialized in one place now.
> 
> Signed-off-by: Maciej W. Rozycki 
> ---
> 
>  This is effectively a follow-up to Nathan's FCR0 fix -- please apply.
> 
>   Maciej

Thanks, applied.

> qemu-mips-hflags.patch
> Index: qemu-git-trunk/target-mips/cpu.h
> ===
> --- qemu-git-trunk.orig/target-mips/cpu.h 2012-06-07 03:15:53.645461055 
> +0100
> +++ qemu-git-trunk/target-mips/cpu.h  2012-06-07 03:18:48.345427587 +0100
> @@ -753,4 +753,53 @@ static inline void cpu_pc_from_tb(CPUMIP
>  env->hflags |= tb->flags & MIPS_HFLAG_BMASK;
>  }
>  
> +static inline void compute_hflags(CPUMIPSState *env)
> +{
> +env->hflags &= ~(MIPS_HFLAG_COP1X | MIPS_HFLAG_64 | MIPS_HFLAG_CP0 |
> + MIPS_HFLAG_F64 | MIPS_HFLAG_FPU | MIPS_HFLAG_KSU |
> + MIPS_HFLAG_UX);
> +if (!(env->CP0_Status & (1 << CP0St_EXL)) &&
> +!(env->CP0_Status & (1 << CP0St_ERL)) &&
> +!(env->hflags & MIPS_HFLAG_DM)) {
> +env->hflags |= (env->CP0_Status >> CP0St_KSU) & MIPS_HFLAG_KSU;
> +}
> +#if defined(TARGET_MIPS64)
> +if (((env->hflags & MIPS_HFLAG_KSU) != MIPS_HFLAG_UM) ||
> +(env->CP0_Status & (1 << CP0St_PX)) ||
> +(env->CP0_Status & (1 << CP0St_UX))) {
> +env->hflags |= MIPS_HFLAG_64;
> +}
> +if (env->CP0_Status & (1 << CP0St_UX)) {
> +env->hflags |= MIPS_HFLAG_UX;
> +}
> +#endif
> +if ((env->CP0_Status & (1 << CP0St_CU0)) ||
> +!(env->hflags & MIPS_HFLAG_KSU)) {
> +env->hflags |= MIPS_HFLAG_CP0;
> +}
> +if (env->CP0_Status & (1 << CP0St_CU1)) {
> +env->hflags |= MIPS_HFLAG_FPU;
> +}
> +if (env->CP0_Status & (1 << CP0St_FR)) {
> +env->hflags |= MIPS_HFLAG_F64;
> +}
> +if (env->insn_flags & ISA_MIPS32R2) {
> +if (env->active_fpu.fcr0 & (1 << FCR0_F64)) {
> +env->hflags |= MIPS_HFLAG_COP1X;
> +}
> +} else if (env->insn_flags & ISA_MIPS32) {
> +if (env->hflags & MIPS_HFLAG_64) {
> +env->hflags |= MIPS_HFLAG_COP1X;
> +}
> +} else if (env->insn_flags & ISA_MIPS4) {
> +/* All supported MIPS IV CPUs use the XX (CU3) to enable
> +   and disable the MIPS IV extensions to the MIPS III ISA.
> +   Some other MIPS IV CPUs ignore the bit, so the check here
> +   would be too restrictive for them.  */
> +if (env->CP0_Status & (1 << CP0St_CU3)) {
> +env->hflags |= MIPS_HFLAG_COP1X;
> +}
> +}
> +}
> +
>  #endif /* !defined (__MIPS_CPU_H__) */
> Index: qemu-git-trunk/target-mips/op_helper.c
> ===
> --- qemu-git-trunk.orig/target-mips/op_helper.c   2012-06-07 
> 03:15:53.645461055 +0100
> +++ qemu-git-trunk/target-mips/op_helper.c2012-06-07 03:18:48.345427587 
> +0100
> @@ -32,55 +32,6 @@
>  static inline void cpu_mips_tlb_flush (CPUMIPSState *env, int flush_global);
>  #endif
>  
> -static inline void compute_hflags(CPUMIPSState *env)
> -{
> -env->hflags &= ~(MIPS_HFLAG_COP1X | MIPS_HFLAG_64 | MIPS_HFLAG_CP0 |
> - MIPS_HFLAG_F64 | MIPS_HFLAG_FPU | MIPS_HFLAG_KSU |
> - MIPS_HFLAG_UX);
> -if (!(env->CP0_Status & (1 << CP0St_EXL)) &&
> -!(env->CP0_Status & (1 << CP0St_ERL)) &&
> -!(env->hflags & MIPS_HFLAG_DM)) {
> -env->hflags |= (env->CP0_Status >> CP0St_KSU) & MIPS_HFLAG_KSU;
> -}
> -#if defined(TARGET_MIPS64)
> -if (((env->hflags & MIPS_HFLAG_KSU) != MIPS_HFLAG_UM) ||
> -(env->CP0_Status & (1 << CP0St_PX)) ||
> -(env->CP0_Status & (1 << CP0St_UX))) {
> -env->hflags |= MIPS_HFLAG_64;
> -}
> -if (env->CP0_Status & (1 << CP0St_UX)) {
> -env->hflags |= MIPS_HFLAG_UX;
> -}
> -#endif
> -if ((env->CP0_Status & (1 << CP0St_CU0)) ||
> -!(env->hflags & MIPS_HFLAG_KSU)) {
> -env->hflags |= MIPS_HFLAG_CP0;
> -}
> -if (env->CP0_Status & (1 << CP0St_CU1)) {
> -env->hflags |= MIPS_HFLAG_FPU;
> -}
> -if (env->CP0_Status & (1 << CP0St_FR)) {
> -env->hflags |= MIPS_HFLAG_F64;
> -}
> -if (env->insn_flags & ISA_MIPS32R2) {
> -if (env->active_fpu.fcr0

Re: [Qemu-devel] [PATCH] [MIPS] Fix order of CVT.PS.S operands

[Aurelien Jarno]

On Mon, Aug 27, 2012 at 09:53:29AM +0100, Richard Sandiford wrote:
> The FS input to CVT.PS.S is the high half and FT is the low half.
> tcg_gen_concat_i32_i64 takes the low half first, so the operands
> were in the wrong order.
> 
> Signed-off-by: Richard Sandiford 
> ---
>  target-mips/translate.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index 06f0ac6..defc021 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -6907,7 +6907,7 @@ static void gen_farith (DisasContext *ctx, enum fopcode 
> op1,
>  
>  gen_load_fpr32(fp32_0, fs);
>  gen_load_fpr32(fp32_1, ft);
> -tcg_gen_concat_i32_i64(fp64, fp32_0, fp32_1);
> +tcg_gen_concat_i32_i64(fp64, fp32_1, fp32_0);
>  tcg_temp_free_i32(fp32_1);
>  tcg_temp_free_i32(fp32_0);
>  gen_store_fpr64(ctx, fp64, fd);

Thanks, applied.


-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net

Re: [Qemu-devel] [PATCH] [MIPS] Fix operands of RECIP2.S and RECIP2.PS

[Aurelien Jarno]

On Mon, Aug 27, 2012 at 09:50:38AM +0100, Richard Sandiford wrote:
> Read the second input operand of RECIP2.S and RECIP2.PS from FT rather
> than FD.  RECIP2.D is already correct.
> 
> Signed-off-by: Richard Sandiford 
> ---
>  target-mips/translate.c |4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index 7104d30..d812986 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -6805,7 +6805,7 @@ static void gen_farith (DisasContext *ctx, enum fopcode 
> op1,
>  TCGv_i32 fp1 = tcg_temp_new_i32();
>  
>  gen_load_fpr32(fp0, fs);
> -gen_load_fpr32(fp1, fd);
> +gen_load_fpr32(fp1, ft);
>  gen_helper_float_recip2_s(fp0, fp0, fp1);
>  tcg_temp_free_i32(fp1);
>  gen_store_fpr32(fp0, fd);
> @@ -7543,7 +7543,7 @@ static void gen_farith (DisasContext *ctx, enum fopcode 
> op1,
>  TCGv_i64 fp1 = tcg_temp_new_i64();
>  
>  gen_load_fpr64(ctx, fp0, fs);
> -gen_load_fpr64(ctx, fp1, fd);
> +gen_load_fpr64(ctx, fp1, ft);
>  gen_helper_float_recip2_ps(fp0, fp0, fp1);
>  tcg_temp_free_i64(fp1);
>  gen_store_fpr64(ctx, fp0, fd);

Thanks, applied.


-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net

Re: [Qemu-devel] [PATCH] [MIPS] Fix operands of RECIP2.S and RECIP2.PS

[Stefan Hajnoczi]

On Mon, Aug 27, 2012 at 9:50 AM, Richard Sandiford
 wrote:
> Read the second input operand of RECIP2.S and RECIP2.PS from FT rather
> than FD.  RECIP2.D is already correct.
>
> Signed-off-by: Richard Sandiford 
> ---
>  target-mips/translate.c |4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index 7104d30..d812986 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -6805,7 +6805,7 @@ static void gen_farith (DisasContext *ctx, enum fopcode 
> op1,
>  TCGv_i32 fp1 = tcg_temp_new_i32();
>
>  gen_load_fpr32(fp0, fs);
> -gen_load_fpr32(fp1, fd);
> +gen_load_fpr32(fp1, ft);
>  gen_helper_float_recip2_s(fp0, fp0, fp1);
>  tcg_temp_free_i32(fp1);
>  gen_store_fpr32(fp0, fd);
> @@ -7543,7 +7543,7 @@ static void gen_farith (DisasContext *ctx, enum fopcode 
> op1,
>  TCGv_i64 fp1 = tcg_temp_new_i64();
>
>  gen_load_fpr64(ctx, fp0, fs);
> -gen_load_fpr64(ctx, fp1, fd);
> +gen_load_fpr64(ctx, fp1, ft);
>  gen_helper_float_recip2_ps(fp0, fp0, fp1);
>  tcg_temp_free_i64(fp1);
>  gen_store_fpr64(ctx, fp0, fd);
> --
> 1.7.7.6

CCing Aurelian for MIPS.  You can look at ./MAINTAINERS to see who
should be CCed.

Stefan

Re: [Qemu-devel] [PATCH] [MIPS] Fix order of CVT.PS.S operands

[Stefan Hajnoczi]

On Mon, Aug 27, 2012 at 9:53 AM, Richard Sandiford
 wrote:
> The FS input to CVT.PS.S is the high half and FT is the low half.
> tcg_gen_concat_i32_i64 takes the low half first, so the operands
> were in the wrong order.
>
> Signed-off-by: Richard Sandiford 
> ---
>  target-mips/translate.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index 06f0ac6..defc021 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -6907,7 +6907,7 @@ static void gen_farith (DisasContext *ctx, enum fopcode 
> op1,
>
>  gen_load_fpr32(fp32_0, fs);
>  gen_load_fpr32(fp32_1, ft);
> -tcg_gen_concat_i32_i64(fp64, fp32_0, fp32_1);
> +tcg_gen_concat_i32_i64(fp64, fp32_1, fp32_0);
>  tcg_temp_free_i32(fp32_1);
>  tcg_temp_free_i32(fp32_0);
>  gen_store_fpr64(ctx, fp64, fd);
> --
> 1.7.7.6

CCing Aurelian for MIPS.  You can look at ./MAINTAINERS to see who
should be CCed.

Stefan

[Qemu-devel] [PATCH] [MIPS] Fix order of CVT.PS.S operands

[Richard Sandiford]

The FS input to CVT.PS.S is the high half and FT is the low half.
tcg_gen_concat_i32_i64 takes the low half first, so the operands
were in the wrong order.

Signed-off-by: Richard Sandiford 
---
 target-mips/translate.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index 06f0ac6..defc021 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -6907,7 +6907,7 @@ static void gen_farith (DisasContext *ctx, enum fopcode 
op1,
 
 gen_load_fpr32(fp32_0, fs);
 gen_load_fpr32(fp32_1, ft);
-tcg_gen_concat_i32_i64(fp64, fp32_0, fp32_1);
+tcg_gen_concat_i32_i64(fp64, fp32_1, fp32_0);
 tcg_temp_free_i32(fp32_1);
 tcg_temp_free_i32(fp32_0);
 gen_store_fpr64(ctx, fp64, fd);
-- 
1.7.7.6

[Qemu-devel] [PATCH] [MIPS] Fix operands of RECIP2.S and RECIP2.PS

[Richard Sandiford]

Read the second input operand of RECIP2.S and RECIP2.PS from FT rather
than FD.  RECIP2.D is already correct.

Signed-off-by: Richard Sandiford 
---
 target-mips/translate.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index 7104d30..d812986 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -6805,7 +6805,7 @@ static void gen_farith (DisasContext *ctx, enum fopcode 
op1,
 TCGv_i32 fp1 = tcg_temp_new_i32();
 
 gen_load_fpr32(fp0, fs);
-gen_load_fpr32(fp1, fd);
+gen_load_fpr32(fp1, ft);
 gen_helper_float_recip2_s(fp0, fp0, fp1);
 tcg_temp_free_i32(fp1);
 gen_store_fpr32(fp0, fd);
@@ -7543,7 +7543,7 @@ static void gen_farith (DisasContext *ctx, enum fopcode 
op1,
 TCGv_i64 fp1 = tcg_temp_new_i64();
 
 gen_load_fpr64(ctx, fp0, fs);
-gen_load_fpr64(ctx, fp1, fd);
+gen_load_fpr64(ctx, fp1, ft);
 gen_helper_float_recip2_ps(fp0, fp0, fp1);
 tcg_temp_free_i64(fp1);
 gen_store_fpr64(ctx, fp0, fd);
-- 
1.7.7.6