Re: [Qemu-devel] [PATCH v3] bugfix: passing reference instead of value

[Paolo Bonzini]

On 02/01/2016 10:06, Stefan Weil wrote:
> Am 02.01.2016 um 09:02 schrieb Cao jin:
>> Fix the bug introduced by 595a4f07: function host_pci_config_read() should be
>> pass-by-reference, not value.
>>
>> Signed-off-by: Cao jin 
>> ---
>> v3 changelog:
>> 1. Remove cpu_to_le32() since the code only runs on X86.
>>
>>  hw/pci-host/piix.c | 8 +---
>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
>> index 715208b..924f0fa 100644
>> --- a/hw/pci-host/piix.c
>> +++ b/hw/pci-host/piix.c
>> @@ -761,7 +761,7 @@ static const IGDHostInfo igd_host_bridge_infos[] = {
>>  {0xa8, 4},  /* SNB: base of GTT stolen memory */
>>  };
>>  
>> -static int host_pci_config_read(int pos, int len, uint32_t val)
>> +static int host_pci_config_read(int pos, int len, uint32_t *val)
>>  {
>>  char path[PATH_MAX];
>>  int config_fd;
>> @@ -784,12 +784,14 @@ static int host_pci_config_read(int pos, int len, 
>> uint32_t val)
>>  ret = -errno;
>>  goto out;
>>  }
>> +
>>  do {
>> -rc = read(config_fd, (uint8_t *), len);
>> +rc = read(config_fd, (uint8_t *)val, len);
> 
> The type cast is not needed here, because read accepts any pointer
> type for the buffer argument.
> 
> While looking at that code, I noticed more potential issues:
> 
> * The open statement needs O_RDWR | O_BINARY, otherwise the code won't
> work on Windows.
> 
> * The len argument can obviously be 2 or 4. Will endianness handling
> work for both cases?

Not sure why this is in pci-host/piix.c, but it's only used on Intel
processors and only on Linux.

Paolo

[Qemu-devel] [PATCH v3] bugfix: passing reference instead of value

[Cao jin]

Fix the bug introduced by 595a4f07: function host_pci_config_read() should be
pass-by-reference, not value.

Signed-off-by: Cao jin 
---
v3 changelog:
1. Remove cpu_to_le32() since the code only runs on X86.

 hw/pci-host/piix.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 715208b..924f0fa 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -761,7 +761,7 @@ static const IGDHostInfo igd_host_bridge_infos[] = {
 {0xa8, 4},  /* SNB: base of GTT stolen memory */
 };
 
-static int host_pci_config_read(int pos, int len, uint32_t val)
+static int host_pci_config_read(int pos, int len, uint32_t *val)
 {
 char path[PATH_MAX];
 int config_fd;
@@ -784,12 +784,14 @@ static int host_pci_config_read(int pos, int len, 
uint32_t val)
 ret = -errno;
 goto out;
 }
+
 do {
-rc = read(config_fd, (uint8_t *), len);
+rc = read(config_fd, (uint8_t *)val, len);
 } while (rc < 0 && (errno == EINTR || errno == EAGAIN));
 if (rc != len) {
 ret = -errno;
 }
+
 out:
 close(config_fd);
 return ret;
@@ -805,7 +807,7 @@ static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev)
 for (i = 0; i < num; i++) {
 pos = igd_host_bridge_infos[i].offset;
 len = igd_host_bridge_infos[i].len;
-rc = host_pci_config_read(pos, len, val);
+rc = host_pci_config_read(pos, len, );
 if (rc) {
 return -ENODEV;
 }
-- 
2.1.0

Re: [Qemu-devel] [PATCH v3] bugfix: passing reference instead of value

[Cao jin]

Hi,
Happy new year:)

On 01/02/2016 05:06 PM, Stefan Weil wrote:

Am 02.01.2016 um 09:02 schrieb Cao jin:

Fix the bug introduced by 595a4f07: function host_pci_config_read() should be
pass-by-reference, not value.

Signed-off-by: Cao jin 
---
v3 changelog:
1. Remove cpu_to_le32() since the code only runs on X86.

  hw/pci-host/piix.c | 8 +---
  1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 715208b..924f0fa 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -761,7 +761,7 @@ static const IGDHostInfo igd_host_bridge_infos[] = {
  {0xa8, 4},  /* SNB: base of GTT stolen memory */
  };

-static int host_pci_config_read(int pos, int len, uint32_t val)
+static int host_pci_config_read(int pos, int len, uint32_t *val)
  {
  char path[PATH_MAX];
  int config_fd;
@@ -784,12 +784,14 @@ static int host_pci_config_read(int pos, int len, 
uint32_t val)
  ret = -errno;
  goto out;
  }
+
  do {
-rc = read(config_fd, (uint8_t *), len);
+rc = read(config_fd, (uint8_t *)val, len);


The type cast is not needed here, because read accepts any pointer
type for the buffer argument.



I guess so, since in function read() prototype, buffer is void *


While looking at that code, I noticed more potential issues:

* The open statement needs O_RDWR | O_BINARY, otherwise the code won't
work on Windows.



I am not quite familiar with things on windows:-[ Let`s see what will 
other people say.



* The len argument can obviously be 2 or 4. Will endianness handling
work for both cases?



I noticed what you find, and after analysing, I think it will works for 
both case:


take vendor ID in config space for example(PCI config space is 
little-endian), assume vendor ID = 0x1234, so in config space, it will 
be laid out as: (lo)34 12(hi).


host_pci_config_read() use read(fd, (uint8_t *)val, len) to get host 
device space value, I guess read() will read it from low address to high 
address, byte by byte(not quite sure about it). So after reading, the 
value in that integer buffer is laid out as: (lo)34,12,0,0(hi)


For (lo)34,12,0,0(hi), a LE machine like X86 will interpret it as number 
0x1234; A BE machine interpret it as 0x3412


since the code only runs on x86, now we have val = 0x1234(len = 2) 
passed to pci_default_write_config(),  it is going to write the value 
into config space like this way: for (i = 0; i < len; val >>= 8, ++i). 
So the endianness is ok.



Regards,
Stefan




--
Yours Sincerely,

Cao Jin

Re: [Qemu-devel] [PATCH v3] bugfix: passing reference instead of value

[Stefan Weil]

Am 02.01.2016 um 09:02 schrieb Cao jin:
> Fix the bug introduced by 595a4f07: function host_pci_config_read() should be
> pass-by-reference, not value.
>
> Signed-off-by: Cao jin 
> ---
> v3 changelog:
> 1. Remove cpu_to_le32() since the code only runs on X86.
>
>  hw/pci-host/piix.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index 715208b..924f0fa 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -761,7 +761,7 @@ static const IGDHostInfo igd_host_bridge_infos[] = {
>  {0xa8, 4},  /* SNB: base of GTT stolen memory */
>  };
>  
> -static int host_pci_config_read(int pos, int len, uint32_t val)
> +static int host_pci_config_read(int pos, int len, uint32_t *val)
>  {
>  char path[PATH_MAX];
>  int config_fd;
> @@ -784,12 +784,14 @@ static int host_pci_config_read(int pos, int len, 
> uint32_t val)
>  ret = -errno;
>  goto out;
>  }
> +
>  do {
> -rc = read(config_fd, (uint8_t *), len);
> +rc = read(config_fd, (uint8_t *)val, len);

The type cast is not needed here, because read accepts any pointer
type for the buffer argument.

While looking at that code, I noticed more potential issues:

* The open statement needs O_RDWR | O_BINARY, otherwise the code won't
work on Windows.

* The len argument can obviously be 2 or 4. Will endianness handling
work for both cases?

Regards,
Stefan




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] Make rtl8139 network interface card compatible with Mac OS 10.4

[Mark Cave-Ayland]

On 02/01/16 04:00, Programmingkid wrote:

>> Well they are returning non-zero values, so that's good. After a bit
>> more poking, something doesn't make sense - in that same file
>> RTL8139::initPCIConfigSpace() claims to set the bus master bit, but if I
>> add tracing to hw/pci/pci-host.c then I don't see any writes to the PCI
>> command register outside of OpenBIOS? Any chance you can add debugging
>> to the before and after values for the reg16 variable which this
>> function claims to set?
>>
>> Also with similar tracing involved, apparently the writes to PCI I/O
>> space in order to access the chip registers are actually going through
>> pci_host_config_write(), i.e. configuration space rather than I/O space
>> which is why they aren't accessing the chip registers. So more digging
>> is required to figure out why this is happening.
>>
> 
> I added code to the initPCIConfigSpace() function that prints every register 
> from 0 to 0x3e. All of the registers returned zero. My patch was fully 
> applied when the driver printed all these values. I even had all the 
> registers set to 0x99 then printed all the registers again. The registers all 
> returned zero again. The driver was able to obtain the correct device, 
> function, and bus numbers. PCI configuration space is not being accessed 
> correctly by this driver. 

After some head scratching chasing this through for several hours
yesterday, I've worked out what is happening and have a patchset for
OpenBIOS which should fix the issue (will post later along with some
other PCI fixes for testing).

This is actually a bug in Darwin in that it doesn't parse the address
space part of the "ranges" property for the /pci node. OpenBIOS adds all
3 spaces to the "ranges" - configuration, I/O and memory compared to
real Macs which only add I/O and memory. Which is generally fine because
each entry contains a 2-bit indicator so that the OS knows which entry
represents each address space.

Except that Darwin evidently doesn't bother parsing the address space
from each "range" entry and blindly assumes that entry 0 is I/O space
and entry 1 is memory space as would be found in a real Mac device tree.
The result of this is that currently I/O accesses go to entry 0 (config
space) and memory accesses go to entry 1 (I/O space) which is why the
RTL8139 happens to work with your patch to force bus mastering.

AFAICT with basic testing here, forcing the /pci node to mimic that of a
real Mac by omitting the configuration space entry seems to fix all PCI
accesses (or at least they get routed to the correct address spaces), I
see an attempt to set the bus master bit in the command register, and I
don't get a reset timeout on startup for the RTL8139 anymore.


ATB,

Mark.

Re: [Qemu-devel] [PATCH] Make rtl8139 network interface card compatible with Mac OS 10.4

[Programmingkid]

On Jan 2, 2016, at 4:39 AM, Mark Cave-Ayland wrote:

> On 02/01/16 04:00, Programmingkid wrote:
> 
>>> Well they are returning non-zero values, so that's good. After a bit
>>> more poking, something doesn't make sense - in that same file
>>> RTL8139::initPCIConfigSpace() claims to set the bus master bit, but if I
>>> add tracing to hw/pci/pci-host.c then I don't see any writes to the PCI
>>> command register outside of OpenBIOS? Any chance you can add debugging
>>> to the before and after values for the reg16 variable which this
>>> function claims to set?
>>> 
>>> Also with similar tracing involved, apparently the writes to PCI I/O
>>> space in order to access the chip registers are actually going through
>>> pci_host_config_write(), i.e. configuration space rather than I/O space
>>> which is why they aren't accessing the chip registers. So more digging
>>> is required to figure out why this is happening.
>>> 
>> 
>> I added code to the initPCIConfigSpace() function that prints every register 
>> from 0 to 0x3e. All of the registers returned zero. My patch was fully 
>> applied when the driver printed all these values. I even had all the 
>> registers set to 0x99 then printed all the registers again. The registers 
>> all returned zero again. The driver was able to obtain the correct device, 
>> function, and bus numbers. PCI configuration space is not being accessed 
>> correctly by this driver. 
> 
> After some head scratching chasing this through for several hours
> yesterday, I've worked out what is happening and have a patchset for
> OpenBIOS which should fix the issue (will post later along with some
> other PCI fixes for testing).
> 
> This is actually a bug in Darwin in that it doesn't parse the address
> space part of the "ranges" property for the /pci node. OpenBIOS adds all
> 3 spaces to the "ranges" - configuration, I/O and memory compared to
> real Macs which only add I/O and memory. Which is generally fine because
> each entry contains a 2-bit indicator so that the OS knows which entry
> represents each address space.
> 
> Except that Darwin evidently doesn't bother parsing the address space
> from each "range" entry and blindly assumes that entry 0 is I/O space
> and entry 1 is memory space as would be found in a real Mac device tree.
> The result of this is that currently I/O accesses go to entry 0 (config
> space) and memory accesses go to entry 1 (I/O space) which is why the
> RTL8139 happens to work with your patch to force bus mastering.
> 
> AFAICT with basic testing here, forcing the /pci node to mimic that of a
> real Mac by omitting the configuration space entry seems to fix all PCI
> accesses (or at least they get routed to the correct address spaces), I
> see an attempt to set the bus master bit in the command register, and I
> don't get a reset timeout on startup for the RTL8139 anymore.


Wow, you figure out stuff fast! Will be waiting for your patches.

Re: [Qemu-devel] [XenGT][IGVT-g] Device model creation failed

[Oleksii Kurochko]

Hello.

I've tried byt_experimental branch and got some another result.
With vgt=1 it failed with some time and got next log:

> [ 4859.380332] vGT info:(create_vgt_instance:118) vm_id=21,
> low_gm_sz=128MB, high_gm_sz=384MB, fence_sz=4, vgt_primary=1
> [ 4859.389889] vGT info:(create_vgt_instance:170) Virtual GTT size:
> 0x20
> [ 4859.393284] VM21 Ring0 context_save_area is allocated at gm(f90d000)
> [ 4859.416542] VM21 Ring1 context_save_area is allocated at gm(f94d000)
> [ 4859.416916] VM21 Ring2 context_save_area is allocated at gm(f98d000)
> [ 4859.417492] vGT info:(create_vgt_instance:206) aperture: [0xc780,
> 0xcf7f] guest [0xc780, 0xcf7f] va(0xc9001820)
> [ 4859.417971] vGT info:(create_vgt_instance:217) GM: [0x780,
> 0xf7f], [0x2800, 0x3fff], guest[0x780, 0xf7f],
> [0x2800, 0x3fff]
> [ 4859.422914] vGT info:(create_vgt_instance:254) filling VGT_PVINFO_PAGE
> for dom21:
> [ 4859.422914]visable_gm_base=0x780, size=0x800
> [ 4859.422914]hidden_gm_base=0x2800, size=0x1800
> [ 4859.422914]fence_base=4, num=4
> [ 4859.423521] vGT info:(vgt_hvm_io_req_handler:795) Received a IOREQ w/o
> vcpu target
> [ 4859.424042] vGT info:(vgt_hvm_io_req_handler:796) Possible a false
> request from event binding
> [ 4859.424555] vGT(1): create debugfs node: virtual_mmio_space
> [ 4859.424713] vGT(1): create debugfs node: shadow_mmio_space
> [ 4859.424883] vGT(1): create debugfs node: frame_buffer_format
> [ 4859.425037] vGT(1): create debugfs node: frame_buffer_format
> [ 4859.425974] vGT info:(vgt_emulation_thread:530) start kthread for VM21
> [ 4859.426231] vGT info:(vgt_propagate_edid:770) EDID_PROPAGATE: Clear
> PORT_A for vm 21
> [ 4859.426508] vGT info:(vgt_propagate_edid:770) EDID_PROPAGATE: Clear
> PORT_B for vm 21
> [ 4859.426759] vGT info:(vgt_propagate_edid:770) EDID_PROPAGATE: Clear
> PORT_C for vm 21
> [ 4859.426992] vGT info:(vgt_propagate_edid:770) EDID_PROPAGATE: Clear
> PORT_D for vm 21
> [ 4859.427224] vGT info:(vgt_propagate_edid:770) EDID_PROPAGATE: Clear
> PORT_E for vm 21
> [ 4859.518800] vGT info:(vgt_vport_connection_store:595) Monitor
> detection: PORT_A  is disconnected
> [ 4859.567255] vGT info:(vgt_vport_connection_store:595) Monitor
> detection: PORT_B  is disconnected
> [ 4859.631077] vGT info:(vgt_vport_connection_store:595) Monitor
> detection: PORT_C  is disconnected
> [ 4859.682043] vGT info:(vgt_vport_connection_store:595) Monitor
> detection: PORT_D  is disconnected
> [ 4859.734223] vGT warning:(pch_adpa_mmio_write:1174)
> HOTPLUG_FORCE_TRIGGER is set while VGA is enabled!
> [ 4859.775194] vGT info:(vgt_vport_connection_store:583) Monitor
> detection: PORT_E  is connected
> [ 4860.111434] add_map: domid=21 gfn_s=0xc7800 mfn_s=0xc7800 nr_mfns=0x8000
> [ 4933.642429] vGT warning:(vgt_hvm_vmem_init:301) VM21:
> vmem_sz=0xf000!
> [ 4943.772119] vGT warning:(vgt_emulate_read:355) vGT: untracked MMIO
> read: vm_id(21), offset=0x4094,len=4, val=0x0!!! base_off=0x0
> [ 4945.801949] vGT warning:(vgt_emulate_read:355) vGT: untracked MMIO
> read: vm_id(21), offset=0x182110,len=4, val=0x0!!! base_off=0x2110
> [ 4945.803030] vGT warning:(vgt_emulate_write:457) vGT: untracked MMIO
> write: vm_id(21), offset=0x182110,len=4, val=0x0!!! base_off=0x2110
> [ 4945.972045] vGT warning:(vgt_emulate_read:355) vGT: untracked MMIO
> read: vm_id(21), offset=0x182110,len=4, val=0x0!!! base_off=0x2110
> [ 4945.972649] vGT warning:(vgt_emulate_write:457) vGT: untracked MMIO
> write: vm_id(21), offset=0x182110,len=4, val=0x1!!! base_off=0x2110
> [ 4946.038157] vGT warning:(vgt_emulate_read:355) vGT: untracked MMIO
> read: vm_id(21), offset=0x61204,len=4, val=0x0!!! base_off=0x0
> [ 4946.039587] vGT warning:(vgt_emulate_write:457) vGT: untracked MMIO
> write: vm_id(21), offset=0x61204,len=4, val=0xabcd!!! base_off=0x0
> [ 4983.876827] vGT info:(vgt_handle_default_event_virt:947) IRQ: VM(21)
> receive event (Blitter Command Streamer MI USER INTERRUPT)
> [ 5023.070461] vGT error:(g2h_gm_range:1660) VM(21): invalid address
> range: g_addr(0x1000), size(0x1000)
> [ 5023.070776] Assert at drivers/xen/vgt/aperture_gm.c line 70
> [ 5023.071049] vGT warning:(mmio_g2h_gmadr:70) Killing VM21
>

Create in next way:

> sudo xl -vvv create ubuntu.hvm
> Parsing config from ubuntu.hvm
> WARNING: specifying "tsc_mode" as an integer is deprecated. Please use the
> named parameter variant. e.g. tsc_mode="default"
> WARNING: ignoring "kernel" directive for HVM guest. Use
> "firmware_override" instead if you really want a non-default firmware
> libxl: debug: libxl_create.c:1230:do_domain_create: ao 0x1da0060: create:
> how=(nil) callback=(nil) poller=0x1da0140
> libxl: debug: libxl_device.c:257:libxl__device_disk_set_backend: Disk
> vdev=hda spec.backend=unknown
> libxl: debug: libxl_device.c:197:disk_try_backend: Disk vdev=hda, backend
> phy unsuitable as phys path not a block device
> libxl: debug: libxl_device.c:212:disk_try_backend: 

Re: [Qemu-devel] [PATCH v3] bugfix: passing reference instead of value

[Michael S. Tsirkin]

On Sat, Jan 02, 2016 at 10:06:10AM +0100, Stefan Weil wrote:
> Am 02.01.2016 um 09:02 schrieb Cao jin:
> > Fix the bug introduced by 595a4f07: function host_pci_config_read() should 
> > be
> > pass-by-reference, not value.
> >
> > Signed-off-by: Cao jin 
> > ---
> > v3 changelog:
> > 1. Remove cpu_to_le32() since the code only runs on X86.
> >
> >  hw/pci-host/piix.c | 8 +---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> > index 715208b..924f0fa 100644
> > --- a/hw/pci-host/piix.c
> > +++ b/hw/pci-host/piix.c
> > @@ -761,7 +761,7 @@ static const IGDHostInfo igd_host_bridge_infos[] = {
> >  {0xa8, 4},  /* SNB: base of GTT stolen memory */
> >  };
> >  
> > -static int host_pci_config_read(int pos, int len, uint32_t val)
> > +static int host_pci_config_read(int pos, int len, uint32_t *val)
> >  {
> >  char path[PATH_MAX];
> >  int config_fd;
> > @@ -784,12 +784,14 @@ static int host_pci_config_read(int pos, int len, 
> > uint32_t val)
> >  ret = -errno;
> >  goto out;
> >  }
> > +
> >  do {
> > -rc = read(config_fd, (uint8_t *), len);
> > +rc = read(config_fd, (uint8_t *)val, len);
> 
> The type cast is not needed here, because read accepts any pointer
> type for the buffer argument.
> 
> While looking at that code, I noticed more potential issues:
> 
> * The open statement needs O_RDWR | O_BINARY, otherwise the code won't
> work on Windows.

I pokes at sysfs, it has no chance to work on windows anyway.

> * The len argument can obviously be 2 or 4. Will endianness handling
> work for both cases?
> 
> Regards,
> Stefan
> 
> 

Re: [Qemu-devel] [PATCH v3] bugfix: passing reference instead of value

[Michael S. Tsirkin]

On Sat, Jan 02, 2016 at 04:02:20PM +0800, Cao jin wrote:
> Fix the bug introduced by 595a4f07: function host_pci_config_read() should be
> pass-by-reference, not value.
> 
> Signed-off-by: Cao jin 
> ---
> v3 changelog:
> 1. Remove cpu_to_le32() since the code only runs on X86.

It really should be le32_to_cpu and a separate patch,
but I think it's preferable to have it there
since people tend to copy code around.

But in any case, before merging any patches in this function I'd like to
hear a response from someone explaining why is this function necessary
at all, since it provably never did anything useful.

> 
>  hw/pci-host/piix.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index 715208b..924f0fa 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -761,7 +761,7 @@ static const IGDHostInfo igd_host_bridge_infos[] = {
>  {0xa8, 4},  /* SNB: base of GTT stolen memory */
>  };
>  
> -static int host_pci_config_read(int pos, int len, uint32_t val)
> +static int host_pci_config_read(int pos, int len, uint32_t *val)
>  {
>  char path[PATH_MAX];
>  int config_fd;
> @@ -784,12 +784,14 @@ static int host_pci_config_read(int pos, int len, 
> uint32_t val)
>  ret = -errno;
>  goto out;
>  }
> +
>  do {
> -rc = read(config_fd, (uint8_t *), len);
> +rc = read(config_fd, (uint8_t *)val, len);
>  } while (rc < 0 && (errno == EINTR || errno == EAGAIN));
>  if (rc != len) {
>  ret = -errno;
>  }
> +
>  out:
>  close(config_fd);
>  return ret;
> @@ -805,7 +807,7 @@ static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev)
>  for (i = 0; i < num; i++) {
>  pos = igd_host_bridge_infos[i].offset;
>  len = igd_host_bridge_infos[i].len;
> -rc = host_pci_config_read(pos, len, val);
> +rc = host_pci_config_read(pos, len, );
>  if (rc) {
>  return -ENODEV;
>  }
> -- 
> 2.1.0
> 
> 

Re: [Qemu-devel] [PATCH] Make rtl8139 network interface card compatible with Mac OS 10.4

[Mark Cave-Ayland]

On 02/01/16 17:08, Programmingkid wrote:

>> After some head scratching chasing this through for several hours
>> yesterday, I've worked out what is happening and have a patchset for
>> OpenBIOS which should fix the issue (will post later along with some
>> other PCI fixes for testing).
>>
>> This is actually a bug in Darwin in that it doesn't parse the address
>> space part of the "ranges" property for the /pci node. OpenBIOS adds all
>> 3 spaces to the "ranges" - configuration, I/O and memory compared to
>> real Macs which only add I/O and memory. Which is generally fine because
>> each entry contains a 2-bit indicator so that the OS knows which entry
>> represents each address space.
>>
>> Except that Darwin evidently doesn't bother parsing the address space
>> from each "range" entry and blindly assumes that entry 0 is I/O space
>> and entry 1 is memory space as would be found in a real Mac device tree.
>> The result of this is that currently I/O accesses go to entry 0 (config
>> space) and memory accesses go to entry 1 (I/O space) which is why the
>> RTL8139 happens to work with your patch to force bus mastering.
>>
>> AFAICT with basic testing here, forcing the /pci node to mimic that of a
>> real Mac by omitting the configuration space entry seems to fix all PCI
>> accesses (or at least they get routed to the correct address spaces), I
>> see an attempt to set the bus master bit in the command register, and I
>> don't get a reset timeout on startup for the RTL8139 anymore.
> 
> 
> Wow, you figure out stuff fast! Will be waiting for your patches.

I've just posted the OpenBIOS patches to the list (see
http://www.openfirmware.info/pipermail/openbios/2016-January/008959.html).
Please test and let me know if this solves your networking problems
against a vanilla QEMU git master checkout - it looks good in the logs,
but I can't quite figure out how to manually bring up a new en0
interface in Darwin from the command line.


ATB,

Mark.

[Qemu-devel] [PATCH v5 0/5] target-tilegx: Implement floating point instructions

[chengang]

From: Chen Gang 

These patches are the normal floating point implementation, instead of
the original temporary one.

It passes building, and gcc testsuite.

Chen Gang (5):
  fpu: softfloat: Add normalize_roundpack_float32 function
  target-tilegx/helper-fshared.h: Add floating point shared function
  target-tilegx/helper-fsingle.c: Implement single floating point
  target-tilegx/helper-fdouble.c: Implement double floating point
  target-tilegx: Integrate floating pointer implementation

 fpu/softfloat.c|  55 ++
 include/fpu/softfloat.h|   8 +
 target-tilegx/Makefile.objs|   3 +-
 target-tilegx/helper-fdouble.c | 384 +
 target-tilegx/helper-fshared.h |  56 ++
 target-tilegx/helper-fsingle.c | 200 +
 target-tilegx/helper.h |  12 ++
 target-tilegx/translate.c  |  66 ++-
 8 files changed, 774 insertions(+), 10 deletions(-)
 create mode 100644 target-tilegx/helper-fdouble.c
 create mode 100644 target-tilegx/helper-fshared.h
 create mode 100644 target-tilegx/helper-fsingle.c

-- 
1.9.3

[Qemu-devel] [PATCH v5 3/5] target-tilegx/helper-fsingle.c: Implement single floating point

[chengang]

From: Chen Gang 

Signed-off-by: Chen Gang 
---
 target-tilegx/helper-fsingle.c | 200 +
 1 file changed, 200 insertions(+)
 create mode 100644 target-tilegx/helper-fsingle.c

diff --git a/target-tilegx/helper-fsingle.c b/target-tilegx/helper-fsingle.c
new file mode 100644
index 000..997d40e
--- /dev/null
+++ b/target-tilegx/helper-fsingle.c
@@ -0,0 +1,200 @@
+/*
+ * QEMU TILE-Gx helpers
+ *
+ *  Copyright (c) 2015 Chen Gang
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see
+ * 
+ */
+
+#include "cpu.h"
+#include "qemu-common.h"
+#include "exec/helper-proto.h"
+#include "fpu/softfloat.h"
+
+#include "helper-fshared.h"
+
+#define IMPL_FULL
+
+/*
+ * FSingle instructions implemenation:
+ *
+ * fsingle_add1 ; calc srca and srcb,
+ *  ; convert float_32 to TileGXFPSFmt result.
+ *  ; move TileGXFPSFmt result to dest.
+ *
+ * fsingle_sub1 ; calc srca and srcb.
+ *  ; convert float_32 to TileGXFPSFmt result.
+ *  ; move TileGXFPSFmt result to dest.
+ *
+ * fsingle_addsub2  ; nop.
+ *
+ * fsingle_mul1 ; calc srca and srcb.
+ *  ; convert float_32 value to TileGXFPSFmt result.
+ *  ; move TileGXFPSFmt result to dest.
+ *
+ * fsingle_mul2 ; move srca to dest.
+ *
+ * fsingle_pack1; nop
+ *
+ * fsingle_pack2; treate srca as TileGXFPSFmt result.
+ *  ; convert TileGXFPSFmt result to float_32 value.
+ *  ; move float_32 value to dest.
+ */
+
+static inline uint32_t get_fsingle_exp(uint64_t n)
+{
+return n & 0xff;
+}
+
+static inline uint32_t get_fsingle_sign(uint64_t n)
+{
+return test_bit(10, );
+}
+
+static inline unsigned int get_fsingle_man(uint64_t n)
+{
+return n >> 32;
+}
+
+#ifdef IMPL_FULL
+
+static uint32_t get_f32_exp(float32 f)
+{
+return extract32(float32_val(f), 23, 8);
+}
+
+static uint32_t get_f32_man(float32 f)
+{
+return float32_val(f) & 0x7f;
+}
+
+static inline void set_fsingle_sign(uint64_t *n)
+{
+set_bit(10, n);
+}
+
+static inline uint64_t create_fsingle_exp(float32 f)
+{
+return  get_f32_exp(f) & 0xff;
+}
+
+static inline uint64_t create_fsingle_man(float32 f)
+{
+if (get_f32_exp(f)) {
+return (uint64_t)get_f32_man(f) << 40 | (1ULL << 63);
+}
+return (uint64_t)get_f32_man(f) << 32;
+}
+
+static uint64_t float32_to_sfmt(float32 f)
+{
+uint64_t sfmt = 0;
+
+if (float32_is_neg(f)) {
+set_fsingle_sign();
+}
+sfmt |= create_fsingle_exp(f);
+sfmt |= create_fsingle_man(f);
+
+return sfmt;
+}
+
+#else
+
+#define TILEGX_F_CALC_CVT   0 /* convert int to fsingle */
+#define TILEGX_F_CALC_NCVT  1 /* Not convertion */
+
+static inline unsigned int get_fsingle_calc(uint64_t n)
+{
+return test_bit(11, );
+}
+
+static inline void set_fsingle_calc(uint64_t *n, uint32_t calc)
+{
+set_bit(11, n);
+}
+
+static uint64_t float32_to_sfmt(float32 f)
+{
+return (uint64_t)float32_val(f) << 32;
+}
+
+#endif
+
+uint64_t helper_fsingle_pack2(uint64_t srca)
+{
+DEC_INIT_FPSTATUS;
+
+#ifndef IMPL_FULL
+if (get_fsingle_calc(srca) == TILEGX_F_CALC_NCVT) {
+return srca >> 32;
+}
+#endif
+return float32_val(normalize_roundpack_float32(get_fsingle_sign(srca),
+   get_fsingle_exp(srca),
+   get_fsingle_man(srca),
+   _status));
+}
+
+static uint64_t main_calc(float32 fsrca, float32 fsrcb,
+  float32 (*calc)(float32, float32, float_status*))
+{
+DEC_INIT_FPSTATUS;
+uint64_t sfmt = float32_to_sfmt(calc(fsrca, fsrcb, _status));
+
+if (float32_eq(fsrca, fsrcb, _status)) {
+sfmt |= create_fsfd_flag_eq();
+} else {
+sfmt |= create_fsfd_flag_ne();
+}
+
+if (float32_lt(fsrca, fsrcb, _status)) {
+sfmt |= create_fsfd_flag_lt();
+}
+if (float32_le(fsrca, fsrcb, _status)) {
+sfmt |= create_fsfd_flag_le();
+}
+
+if (float32_lt(fsrcb, fsrca, _status)) {
+sfmt |= create_fsfd_flag_gt();
+}
+if (float32_le(fsrcb, fsrca, _status)) {
+   

[Qemu-devel] [PATCH v5 4/5] target-tilegx/helper-fdouble.c: Implement double floating point

[chengang]

From: Chen Gang 

Signed-off-by: Chen Gang 
---
 target-tilegx/helper-fdouble.c | 383 +
 1 file changed, 384 insertions(+)
 create mode 100644 target-tilegx/helper-fdouble.c

diff --git a/target-tilegx/helper-fdouble.c b/target-tilegx/helper-fdouble.c
new file mode 100644
index 000..9aab3ac
--- /dev/null
+++ b/target-tilegx/helper-fdouble.c
@@ -0,0 +1,383 @@
+/*
+ * QEMU TILE-Gx helpers
+ *
+ *  Copyright (c) 2015 Chen Gang
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see
+ * 
+ */
+
+#include "cpu.h"
+#include "qemu-common.h"
+#include "exec/helper-proto.h"
+#include "fpu/softfloat.h"
+
+#include "helper-fshared.h"
+
+/*
+ * FDouble instructions implemenation:
+ *
+ * fdouble_unpack_min   ; srca and srcb are float_64 value.
+ *  ; get the min absolute value's mantissa.
+ *  ; move "mantissa >> (exp_max - exp_min)" to dest.
+ *
+ * fdouble_unpack_max   ; srca and srcb are float_64 value.
+ *  ; get the max absolute value's mantissa.
+ *  ; move mantissa to dest.
+ *
+ * fdouble_add_flags; srca and srcb are float_64 value.
+ *  ; calc exp (exp_max), sign, and comp bits for flags.
+ *  ; set addsub bit to flags and move flags to dest.
+ *
+ * fdouble_sub_flags; srca and srcb are float_64 value.
+ *  ; calc exp (exp_max), sign, and comp bits for flags.
+ *  ; set addsub bit to flags and move flags to dest.
+ *
+ * fdouble_addsub:  ; dest, srca (max, min mantissa), and srcb (flags).
+ *  ; "dest +/- srca" depend on the add/sub bit of flags.
+ *  ; move result mantissa to dest.
+ *
+ * fdouble_mul_flags:   ; srca and srcb are float_64 value.
+ *  ; calc sign (xor), exp (min + max), and comp bits.
+ *  ; mix sign, exp, and comp bits as flags to dest.
+ *
+ * fdouble_pack1; move srcb (flags) to dest.
+ *
+ * fdouble_pack2; srca, srcb (high, low mantissa), and dest (flags)
+ *  ; normalize and pack result from srca, srcb, and dest.
+ *  ; move result to dest.
+ */
+
+#define TILEGX_F_EXP_DZERO  0x3ff /* Zero exp for double 11-bits */
+#define TILEGX_F_EXP_DMAX   0x7fe /* max exp for double 11-bits */
+#define TILEGX_F_EXP_DUF0x1000/* underflow exp bit for double */
+
+#define TILEGX_F_MAN_HBIT   (1ULL << 59)
+
+#define TILEGX_F_CALC_ADD   1 /* Perform absolute add operation */
+#define TILEGX_F_CALC_SUB   2 /* Perform absolute sub operation */
+#define TILEGX_F_CALC_MUL   3 /* Perform absolute mul operation */
+
+static uint32_t get_f64_exp(float64 d)
+{
+return extract64(float64_val(d), 52, 11);
+}
+
+static void set_f64_exp(float64 *d, uint32_t exp)
+{
+*d = make_float64(deposit64(float64_val(*d), 52, 11, exp));
+}
+
+static uint64_t get_f64_man(float64 d)
+{
+return extract64(float64_val(d), 0, 52);
+}
+
+static uint64_t fr_to_man(float64 d)
+{
+uint64_t val = get_f64_man(d) << 7;
+
+if (get_f64_exp(d)) {
+val |= TILEGX_F_MAN_HBIT; /* Restore HBIT for the next calculation */
+}
+
+return val;
+}
+
+static uint64_t get_fdouble_man(uint64_t n)
+{
+return extract64(n, 0, 60);
+}
+
+static void set_fdouble_man(uint64_t *n, uint64_t man)
+{
+*n = deposit64(*n, 0, 60, man);
+}
+
+static uint64_t get_fdouble_man_of(uint64_t n)
+{
+return test_bit(60, );
+}
+
+static void clear_fdouble_man_of(uint64_t *n)
+{
+return clear_bit(60, n);
+}
+
+static uint32_t get_fdouble_nan(uint64_t n)
+{
+return test_bit(24, );
+}
+
+static void set_fdouble_nan(uint64_t *n)
+{
+set_bit(24, n);
+}
+
+static uint32_t get_fdouble_inf(uint64_t n)
+{
+return test_bit(23, );
+}
+
+static void set_fdouble_inf(uint64_t *n)
+{
+set_bit(23, n);
+}
+
+static uint32_t get_fdouble_calc(uint64_t n)
+{
+return extract32(n, 21, 2);
+}
+
+static void set_fdouble_calc(uint64_t *n, uint32_t calc)
+{
+*n = deposit64(*n, 21, 2, calc);
+}
+
+static uint32_t get_fdouble_sign(uint64_t n)
+{
+return test_bit(20, );
+}
+
+static void set_fdouble_sign(uint64_t *n)
+{
+set_bit(20, n);
+}
+
+static uint32_t get_fdouble_vexp(uint64_t n)

[Qemu-devel] [PATCH v5 5/5] target-tilegx: Integrate floating pointer implementation

[chengang]

From: Chen Gang 

It passes normal building, and gcc testsuite.

Signed-off-by: Chen Gang 
---
 target-tilegx/Makefile.objs |  3 ++-
 target-tilegx/helper.h  | 12 +
 target-tilegx/translate.c   | 66 ++---
 3 files changed, 71 insertions(+), 10 deletions(-)

diff --git a/target-tilegx/Makefile.objs b/target-tilegx/Makefile.objs
index 0db778f..136ad60 100644
--- a/target-tilegx/Makefile.objs
+++ b/target-tilegx/Makefile.objs
@@ -1 +1,2 @@
-obj-y += cpu.o translate.o helper.o simd_helper.o
+obj-y += cpu.o translate.o helper.o simd_helper.o \
+   helper-fsingle.o helper-fdouble.o
diff --git a/target-tilegx/helper.h b/target-tilegx/helper.h
index 9281d0f..3471fe3 100644
--- a/target-tilegx/helper.h
+++ b/target-tilegx/helper.h
@@ -24,3 +24,15 @@ DEF_HELPER_FLAGS_2(v1shrs, TCG_CALL_NO_RWG_SE, i64, i64, i64)
 DEF_HELPER_FLAGS_2(v2shl, TCG_CALL_NO_RWG_SE, i64, i64, i64)
 DEF_HELPER_FLAGS_2(v2shru, TCG_CALL_NO_RWG_SE, i64, i64, i64)
 DEF_HELPER_FLAGS_2(v2shrs, TCG_CALL_NO_RWG_SE, i64, i64, i64)
+
+DEF_HELPER_2(fsingle_add1, i64, i64, i64)
+DEF_HELPER_2(fsingle_sub1, i64, i64, i64)
+DEF_HELPER_2(fsingle_mul1, i64, i64, i64)
+DEF_HELPER_1(fsingle_pack2, i64, i64)
+DEF_HELPER_2(fdouble_unpack_min, i64, i64, i64)
+DEF_HELPER_2(fdouble_unpack_max, i64, i64, i64)
+DEF_HELPER_2(fdouble_add_flags, i64, i64, i64)
+DEF_HELPER_2(fdouble_sub_flags, i64, i64, i64)
+DEF_HELPER_3(fdouble_addsub, i64, i64, i64, i64)
+DEF_HELPER_2(fdouble_mul_flags, i64, i64, i64)
+DEF_HELPER_3(fdouble_pack2, i64, i64, i64, i64)
diff --git a/target-tilegx/translate.c b/target-tilegx/translate.c
index 354f25a..924eece 100644
--- a/target-tilegx/translate.c
+++ b/target-tilegx/translate.c
@@ -597,6 +597,11 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned 
opext,
 }
 qemu_log_mask(CPU_LOG_TB_IN_ASM, "%s %s", mnemonic, reg_names[srca]);
 return ret;
+
+case OE_RR_X0(FSINGLE_PACK1):
+case OE_RR_Y0(FSINGLE_PACK1):
+mnemonic = "fsingle_pack1";
+goto done2;
 }
 
 tdest = dest_gr(dc, dest);
@@ -613,9 +618,6 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned 
opext,
 gen_helper_cnttz(tdest, tsrca);
 mnemonic = "cnttz";
 break;
-case OE_RR_X0(FSINGLE_PACK1):
-case OE_RR_Y0(FSINGLE_PACK1):
-return TILEGX_EXCP_OPCODE_UNIMPLEMENTED;
 case OE_RR_X1(LD1S):
 memop = MO_SB;
 mnemonic = "ld1s"; /* prefetch_l1_fault */
@@ -734,6 +736,7 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned 
opext,
 return TILEGX_EXCP_OPCODE_UNKNOWN;
 }
 
+done2:
 qemu_log_mask(CPU_LOG_TB_IN_ASM, "%s %s, %s", mnemonic,
   reg_names[dest], reg_names[srca]);
 return ret;
@@ -742,13 +745,21 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned 
opext,
 static TileExcp gen_rrr_opcode(DisasContext *dc, unsigned opext,
unsigned dest, unsigned srca, unsigned srcb)
 {
-TCGv tdest = dest_gr(dc, dest);
-TCGv tsrca = load_gr(dc, srca);
-TCGv tsrcb = load_gr(dc, srcb);
+TCGv tdest, tsrca, tsrcb;
 TCGv t0;
 const char *mnemonic;
 
 switch (opext) {
+case OE_RRR(FSINGLE_ADDSUB2, 0, X0):
+mnemonic = "fsingle_addsub2";
+goto done2;
+}
+
+tdest = dest_gr(dc, dest);
+tsrca = load_gr(dc, srca);
+tsrcb = load_gr(dc, srcb);
+
+switch (opext) {
 case OE_RRR(ADDXSC, 0, X0):
 case OE_RRR(ADDXSC, 0, X1):
 gen_saturate_op(tdest, tsrca, tsrcb, tcg_gen_add_tl);
@@ -906,14 +917,37 @@ static TileExcp gen_rrr_opcode(DisasContext *dc, unsigned 
opext,
 mnemonic = "exch";
 break;
 case OE_RRR(FDOUBLE_ADDSUB, 0, X0):
+gen_helper_fdouble_addsub(tdest, load_gr(dc, dest), tsrca, tsrcb);
+mnemonic = "fdouble_addsub";
+break;
 case OE_RRR(FDOUBLE_ADD_FLAGS, 0, X0):
+gen_helper_fdouble_add_flags(tdest, tsrca, tsrcb);
+mnemonic = "fdouble_add_flags";
+break;
 case OE_RRR(FDOUBLE_MUL_FLAGS, 0, X0):
+gen_helper_fdouble_mul_flags(tdest, tsrca, tsrcb);
+mnemonic = "fdouble_mul_flags";
+break;
 case OE_RRR(FDOUBLE_PACK1, 0, X0):
+tcg_gen_mov_i64(tdest, tsrcb);
+mnemonic = "fdouble_pack1";
+break;
 case OE_RRR(FDOUBLE_PACK2, 0, X0):
+gen_helper_fdouble_pack2(tdest, load_gr(dc, dest), tsrca, tsrcb);
+mnemonic = "fdouble_pack2";
+break;
 case OE_RRR(FDOUBLE_SUB_FLAGS, 0, X0):
+gen_helper_fdouble_sub_flags(tdest, tsrca, tsrcb);
+mnemonic = "fdouble_sub_flags";
+break;
 case OE_RRR(FDOUBLE_UNPACK_MAX, 0, X0):
+gen_helper_fdouble_unpack_max(tdest, tsrca, tsrcb);
+mnemonic = "fdouble_unpack_max";
+break;
 case OE_RRR(FDOUBLE_UNPACK_MIN, 0, X0):
-return TILEGX_EXCP_OPCODE_UNIMPLEMENTED;
+

[Qemu-devel] [PATCH v5 1/5] fpu: softfloat: Add normalize_roundpack_float32 function

[chengang]

From: Chen Gang 

It is based on (u)int32_to_float32 function to support float32 packing.

Signed-off-by: Chen Gang 
---
 fpu/softfloat.c | 55 +
 include/fpu/softfloat.h |  8 +++
 2 files changed, 63 insertions(+)

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index f1170fe..dba8566 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -7080,6 +7080,61 @@ float64 uint32_to_float64(uint32_t a, float_status 
*status)
 return int64_to_float64(a, status);
 }
 
+/*
+ * The mantissa contents the hide bit, e.g. exp: 0x9e with sig: 1 means 1.0f.
+ *
+ * It references from int32_to_float32() and uint32_to_float32()
+ */
+float32 normalize_roundpack_float32(flag sign, int_fast16_t exp, uint32_t sig,
+float_status *status)
+{
+uint64_t absa = sig;
+int8_t scount;
+
+if (exp >= 0xff) {
+return packFloat32(sign, 0xFF, 0);
+} else if (exp <= 0) {
+shift32RightJamming(sig, 0 - exp, );
+return packFloat32(sign, 0, sig);
+}
+
+if (sign) {
+if (sig & 0x7FFF) {
+return normalizeRoundAndPackFloat32(1, exp - 2, sig, status);
+}
+if (sig) {
+return packFloat32(1, exp, 0);
+} else {
+return float32_zero;
+}
+}
+
+if (!sig) {
+return float32_zero;
+}
+
+scount = countLeadingZeros64(absa) - 40;
+if (scount >= 0) {
+exp -= 7 + scount + 2;
+if (exp <= 0) {
+return packFloat32(0, 0, absa);
+}
+return packFloat32(0, exp, absa << scount);
+}
+
+scount += 7;
+exp -= scount + 2;
+if (exp <= 0) {
+return packFloat32(0, 0, absa);
+}
+if (scount < 0) {
+shift64RightJamming(absa, 0 - scount, );
+} else {
+absa <<= scount;
+}
+return roundAndPackFloat32(0, exp, absa, status);
+}
+
 uint32 float32_to_uint32(float32 a, float_status *status)
 {
 int64_t v;
diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index ded34eb..4995a15 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -422,6 +422,14 @@ int float32_is_signaling_nan( float32 );
 float32 float32_maybe_silence_nan( float32 );
 float32 float32_scalbn(float32, int, float_status *status);
 
+/*
+ * The mantissa contents the hide bit, e.g. exp: 0x9e with sig: 1 means 1.0f.
+ *
+ * It references from int32_to_float32() and uint32_to_float32()
+ */
+float32 normalize_roundpack_float32(flag sign, int_fast16_t exp, uint32_t sig,
+float_status *status);
+
 static inline float32 float32_abs(float32 a)
 {
 /* Note that abs does *not* handle NaN specially, nor does
-- 
1.9.3

[Qemu-devel] [PATCH v5 2/5] target-tilegx/helper-fshared.h: Add floating point shared function

[chengang]

From: Chen Gang 

It is used by fsingle and fdouble helpers.

Signed-off-by: Chen Gang 
---
 target-tilegx/helper-fshared.h | 56 ++
 1 file changed, 56 insertions(+)
 create mode 100644 target-tilegx/helper-fshared.h

diff --git a/target-tilegx/helper-fshared.h b/target-tilegx/helper-fshared.h
new file mode 100644
index 000..bcd673e
--- /dev/null
+++ b/target-tilegx/helper-fshared.h
@@ -0,0 +1,56 @@
+/*
+ *  TILE-Gx virtual Floating point shared functions
+ *
+ *  Copyright (c) 2015 Chen Gang
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ */
+
+#define DEC_INIT_FPSTATUS \
+float_status fp_status = {.float_rounding_mode = float_round_nearest_even};
+
+static inline uint64_t create_fsfd_flag_un(void)
+{
+return 1 << 25;
+}
+
+static inline uint64_t create_fsfd_flag_lt(void)
+{
+return 1 << 26;
+}
+
+static inline uint64_t create_fsfd_flag_le(void)
+{
+return 1 << 27;
+}
+
+static inline uint64_t create_fsfd_flag_gt(void)
+{
+return 1 << 28;
+}
+
+static inline uint64_t create_fsfd_flag_ge(void)
+{
+return 1 << 29;
+}
+
+static inline uint64_t create_fsfd_flag_eq(void)
+{
+return 1 << 30;
+}
+
+static inline uint64_t create_fsfd_flag_ne(void)
+{
+return 1ULL << 31;
+}
-- 
1.9.3

Re: [Qemu-devel] [PATCH v5 1/5] fpu: softfloat: Add normalize_roundpack_float32 function

[Chen Gang]

For sig == 0 case, the original implementation is incorrect (although it
passes gcc testsuite), it needs to consider about sign for float_zero.

The related fix diff for it is below. After patches v5 are finished
reviewing, I shall merge the fix diff below to patch v6, next.

Thanks.

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index dba8566..5ad8bb5 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -7098,19 +7098,15 @@ float32 normalize_roundpack_float32(flag sign, 
int_fast16_t exp, uint32_t sig,
 return packFloat32(sign, 0, sig);
 }
 
+if (!sig) {
+return float32_set_sign(float32_zero, sign);
+}
+
 if (sign) {
 if (sig & 0x7FFF) {
 return normalizeRoundAndPackFloat32(1, exp - 2, sig, status);
 }
-if (sig) {
-return packFloat32(1, exp, 0);
-} else {
-return float32_zero;
-}
-}
-
-if (!sig) {
-return float32_zero;
+return packFloat32(1, exp, 0);
 }
 
 scount = countLeadingZeros64(absa) - 40;

On 1/3/16 06:25, cheng...@emindsoft.com.cn wrote:
> From: Chen Gang 
> 
> It is based on (u)int32_to_float32 function to support float32 packing.
> 
> Signed-off-by: Chen Gang 
> ---
>  fpu/softfloat.c | 55 
> +
>  include/fpu/softfloat.h |  8 +++
>  2 files changed, 63 insertions(+)
> 
> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
> index f1170fe..dba8566 100644
> --- a/fpu/softfloat.c
> +++ b/fpu/softfloat.c
> @@ -7080,6 +7080,61 @@ float64 uint32_to_float64(uint32_t a, float_status 
> *status)
>  return int64_to_float64(a, status);
>  }
>  
> +/*
> + * The mantissa contents the hide bit, e.g. exp: 0x9e with sig: 1 means 1.0f.
> + *
> + * It references from int32_to_float32() and uint32_to_float32()
> + */
> +float32 normalize_roundpack_float32(flag sign, int_fast16_t exp, uint32_t 
> sig,
> +float_status *status)
> +{
> +uint64_t absa = sig;
> +int8_t scount;
> +
> +if (exp >= 0xff) {
> +return packFloat32(sign, 0xFF, 0);
> +} else if (exp <= 0) {
> +shift32RightJamming(sig, 0 - exp, );
> +return packFloat32(sign, 0, sig);
> +}
> +
> +if (sign) {
> +if (sig & 0x7FFF) {
> +return normalizeRoundAndPackFloat32(1, exp - 2, sig, status);
> +}
> +if (sig) {
> +return packFloat32(1, exp, 0);
> +} else {
> +return float32_zero;
> +}
> +}
> +
> +if (!sig) {
> +return float32_zero;
> +}
> +
> +scount = countLeadingZeros64(absa) - 40;
> +if (scount >= 0) {
> +exp -= 7 + scount + 2;
> +if (exp <= 0) {
> +return packFloat32(0, 0, absa);
> +}
> +return packFloat32(0, exp, absa << scount);
> +}
> +
> +scount += 7;
> +exp -= scount + 2;
> +if (exp <= 0) {
> +return packFloat32(0, 0, absa);
> +}
> +if (scount < 0) {
> +shift64RightJamming(absa, 0 - scount, );
> +} else {
> +absa <<= scount;
> +}
> +return roundAndPackFloat32(0, exp, absa, status);
> +}
> +
>  uint32 float32_to_uint32(float32 a, float_status *status)
>  {
>  int64_t v;
> diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
> index ded34eb..4995a15 100644
> --- a/include/fpu/softfloat.h
> +++ b/include/fpu/softfloat.h
> @@ -422,6 +422,14 @@ int float32_is_signaling_nan( float32 );
>  float32 float32_maybe_silence_nan( float32 );
>  float32 float32_scalbn(float32, int, float_status *status);
>  
> +/*
> + * The mantissa contents the hide bit, e.g. exp: 0x9e with sig: 1 means 1.0f.
> + *
> + * It references from int32_to_float32() and uint32_to_float32()
> + */
> +float32 normalize_roundpack_float32(flag sign, int_fast16_t exp, uint32_t 
> sig,
> +float_status *status);
> +
>  static inline float32 float32_abs(float32 a)
>  {
>  /* Note that abs does *not* handle NaN specially, nor does
> 

-- 
Chen Gang (陈刚)

Open, share, and attitude like air, water, and life which God blessed