|
From: | Cao jin |
Subject: | Re: [Qemu-devel] [PATCH v2] bugfix: passing reference instead of value |
Date: | Wed, 30 Dec 2015 10:57:52 +0800 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 |
Hi Paolo On 12/30/2015 12:25 AM, Paolo Bonzini wrote:
Separated from previous "igd-passthru convert to realize" patch. Since these two don`t have dependency, can send it solely. Not test since it is easy to find out if reading carefully, just compiled.This patch works but the added conversion to LE is conceptually wrong. The conversion from LE to CPU and vice versa is already done by host_pci_config_read and pci_default_write_config. You don't have it in host_pci_config_read because the code is not portable and x86 is little-endian.
Do you mean, this code section(or this igd passthru device)will only run on x86? If it is, yes, the cpu_to_le32() is not necessary. If it has opportunity to run on BE, I think it is necessary, right?
The reason I add the conversion is that I am not sure whether it will run on a BE machine or not, and it will no bad to LE machine like x86.
Generally, functions that accept/return integers take care themselves of endianness conversions. Paolohw/pci-host/piix.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c index 715208b..a9cb983 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 *)&val, 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,11 +807,11 @@ 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, &val); if (rc) { return -ENODEV; } - pci_default_write_config(pci_dev, pos, val, len); + pci_default_write_config(pci_dev, pos, cpu_to_le32(val), len); } return 0;.
-- Yours Sincerely, Cao Jin
[Prev in Thread] | Current Thread | [Next in Thread] |