* [Qemu-devel] [PATCH] pcnet: Fix sign extension: make ipxe work with >2G RAM
@ 2011-03-15 16:47 Alex Williamson
2011-03-17 9:08 ` [Qemu-devel] " Stefan Hajnoczi
2011-04-01 20:35 ` [Qemu-devel] " Aurelien Jarno
0 siblings, 2 replies; 3+ messages in thread
From: Alex Williamson @ 2011-03-15 16:47 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.williamson, mcb30, stefanha
From: Michael Brown <mcb30@ipxe.org>
The problem is with definitions in hw/pcnet.c such as:
#define CSR_CRDA(S) ((S)->csr[28] | ((S)->csr[29] << 16))
"(S)->csr[29]" is a uint16_t, but "(S)->csr[29] << 16" gets promoted to
int, so the overall CSR_CRDA(s) is a (signed) int rather than a uint32_t.
This then gets assigned to a uint64_t using
target_phys_addr_t crda = CSR_CRDA(s);
so when (S)->csr[29] has the high bit set, we end up with
crda=0xffffffffxxxxxxxx.
From: Michael Brown <mcb30@ipxe.org>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
hw/pcnet.c | 30 +++++++++++++++---------------
1 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/hw/pcnet.c b/hw/pcnet.c
index 6dfdcc4..e961d14 100644
--- a/hw/pcnet.c
+++ b/hw/pcnet.c
@@ -113,23 +113,23 @@ struct qemu_ether_header {
#define CSR_XMTRL(S) ((S)->csr[78])
#define CSR_MISSC(S) ((S)->csr[112])
-#define CSR_IADR(S) ((S)->csr[ 1] | ((S)->csr[ 2] << 16))
-#define CSR_CRBA(S) ((S)->csr[18] | ((S)->csr[19] << 16))
-#define CSR_CXBA(S) ((S)->csr[20] | ((S)->csr[21] << 16))
-#define CSR_NRBA(S) ((S)->csr[22] | ((S)->csr[23] << 16))
-#define CSR_BADR(S) ((S)->csr[24] | ((S)->csr[25] << 16))
-#define CSR_NRDA(S) ((S)->csr[26] | ((S)->csr[27] << 16))
-#define CSR_CRDA(S) ((S)->csr[28] | ((S)->csr[29] << 16))
-#define CSR_BADX(S) ((S)->csr[30] | ((S)->csr[31] << 16))
-#define CSR_NXDA(S) ((S)->csr[32] | ((S)->csr[33] << 16))
-#define CSR_CXDA(S) ((S)->csr[34] | ((S)->csr[35] << 16))
-#define CSR_NNRD(S) ((S)->csr[36] | ((S)->csr[37] << 16))
-#define CSR_NNXD(S) ((S)->csr[38] | ((S)->csr[39] << 16))
-#define CSR_PXDA(S) ((S)->csr[60] | ((S)->csr[61] << 16))
-#define CSR_NXBA(S) ((S)->csr[64] | ((S)->csr[65] << 16))
+#define CSR_IADR(S) ((S)->csr[ 1] | ((uint32_t)(S)->csr[ 2] << 16))
+#define CSR_CRBA(S) ((S)->csr[18] | ((uint32_t)(S)->csr[19] << 16))
+#define CSR_CXBA(S) ((S)->csr[20] | ((uint32_t)(S)->csr[21] << 16))
+#define CSR_NRBA(S) ((S)->csr[22] | ((uint32_t)(S)->csr[23] << 16))
+#define CSR_BADR(S) ((S)->csr[24] | ((uint32_t)(S)->csr[25] << 16))
+#define CSR_NRDA(S) ((S)->csr[26] | ((uint32_t)(S)->csr[27] << 16))
+#define CSR_CRDA(S) ((S)->csr[28] | ((uint32_t)(S)->csr[29] << 16))
+#define CSR_BADX(S) ((S)->csr[30] | ((uint32_t)(S)->csr[31] << 16))
+#define CSR_NXDA(S) ((S)->csr[32] | ((uint32_t)(S)->csr[33] << 16))
+#define CSR_CXDA(S) ((S)->csr[34] | ((uint32_t)(S)->csr[35] << 16))
+#define CSR_NNRD(S) ((S)->csr[36] | ((uint32_t)(S)->csr[37] << 16))
+#define CSR_NNXD(S) ((S)->csr[38] | ((uint32_t)(S)->csr[39] << 16))
+#define CSR_PXDA(S) ((S)->csr[60] | ((uint32_t)(S)->csr[61] << 16))
+#define CSR_NXBA(S) ((S)->csr[64] | ((uint32_t)(S)->csr[65] << 16))
#define PHYSADDR(S,A) \
- (BCR_SSIZE32(S) ? (A) : (A) | ((0xff00 & (uint32_t)(s)->csr[2])<<16))
+ (BCR_SSIZE32(S) ? (A) : (A) | ((0xff00 & (uint32_t)(S)->csr[2])<<16))
struct pcnet_initblk16 {
uint16_t mode;
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [Qemu-devel] Re: [PATCH] pcnet: Fix sign extension: make ipxe work with >2G RAM
2011-03-15 16:47 [Qemu-devel] [PATCH] pcnet: Fix sign extension: make ipxe work with >2G RAM Alex Williamson
@ 2011-03-17 9:08 ` Stefan Hajnoczi
2011-04-01 20:35 ` [Qemu-devel] " Aurelien Jarno
1 sibling, 0 replies; 3+ messages in thread
From: Stefan Hajnoczi @ 2011-03-17 9:08 UTC (permalink / raw)
To: Alex Williamson; +Cc: qemu-devel, mcb30
On Tue, Mar 15, 2011 at 10:47:22AM -0600, Alex Williamson wrote:
> From: Michael Brown <mcb30@ipxe.org>
>
> The problem is with definitions in hw/pcnet.c such as:
>
> #define CSR_CRDA(S) ((S)->csr[28] | ((S)->csr[29] << 16))
>
> "(S)->csr[29]" is a uint16_t, but "(S)->csr[29] << 16" gets promoted to
> int, so the overall CSR_CRDA(s) is a (signed) int rather than a uint32_t.
>
> This then gets assigned to a uint64_t using
>
> target_phys_addr_t crda = CSR_CRDA(s);
>
> so when (S)->csr[29] has the high bit set, we end up with
> crda=0xffffffffxxxxxxxx.
>
> From: Michael Brown <mcb30@ipxe.org>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>
> hw/pcnet.c | 30 +++++++++++++++---------------
> 1 files changed, 15 insertions(+), 15 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] pcnet: Fix sign extension: make ipxe work with >2G RAM
2011-03-15 16:47 [Qemu-devel] [PATCH] pcnet: Fix sign extension: make ipxe work with >2G RAM Alex Williamson
2011-03-17 9:08 ` [Qemu-devel] " Stefan Hajnoczi
@ 2011-04-01 20:35 ` Aurelien Jarno
1 sibling, 0 replies; 3+ messages in thread
From: Aurelien Jarno @ 2011-04-01 20:35 UTC (permalink / raw)
To: Alex Williamson; +Cc: qemu-devel, stefanha, mcb30
On Tue, Mar 15, 2011 at 10:47:22AM -0600, Alex Williamson wrote:
> From: Michael Brown <mcb30@ipxe.org>
>
> The problem is with definitions in hw/pcnet.c such as:
>
> #define CSR_CRDA(S) ((S)->csr[28] | ((S)->csr[29] << 16))
>
> "(S)->csr[29]" is a uint16_t, but "(S)->csr[29] << 16" gets promoted to
> int, so the overall CSR_CRDA(s) is a (signed) int rather than a uint32_t.
>
> This then gets assigned to a uint64_t using
>
> target_phys_addr_t crda = CSR_CRDA(s);
>
> so when (S)->csr[29] has the high bit set, we end up with
> crda=0xffffffffxxxxxxxx.
>
> From: Michael Brown <mcb30@ipxe.org>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>
> hw/pcnet.c | 30 +++++++++++++++---------------
> 1 files changed, 15 insertions(+), 15 deletions(-)
Thanks, applied.
> diff --git a/hw/pcnet.c b/hw/pcnet.c
> index 6dfdcc4..e961d14 100644
> --- a/hw/pcnet.c
> +++ b/hw/pcnet.c
> @@ -113,23 +113,23 @@ struct qemu_ether_header {
> #define CSR_XMTRL(S) ((S)->csr[78])
> #define CSR_MISSC(S) ((S)->csr[112])
>
> -#define CSR_IADR(S) ((S)->csr[ 1] | ((S)->csr[ 2] << 16))
> -#define CSR_CRBA(S) ((S)->csr[18] | ((S)->csr[19] << 16))
> -#define CSR_CXBA(S) ((S)->csr[20] | ((S)->csr[21] << 16))
> -#define CSR_NRBA(S) ((S)->csr[22] | ((S)->csr[23] << 16))
> -#define CSR_BADR(S) ((S)->csr[24] | ((S)->csr[25] << 16))
> -#define CSR_NRDA(S) ((S)->csr[26] | ((S)->csr[27] << 16))
> -#define CSR_CRDA(S) ((S)->csr[28] | ((S)->csr[29] << 16))
> -#define CSR_BADX(S) ((S)->csr[30] | ((S)->csr[31] << 16))
> -#define CSR_NXDA(S) ((S)->csr[32] | ((S)->csr[33] << 16))
> -#define CSR_CXDA(S) ((S)->csr[34] | ((S)->csr[35] << 16))
> -#define CSR_NNRD(S) ((S)->csr[36] | ((S)->csr[37] << 16))
> -#define CSR_NNXD(S) ((S)->csr[38] | ((S)->csr[39] << 16))
> -#define CSR_PXDA(S) ((S)->csr[60] | ((S)->csr[61] << 16))
> -#define CSR_NXBA(S) ((S)->csr[64] | ((S)->csr[65] << 16))
> +#define CSR_IADR(S) ((S)->csr[ 1] | ((uint32_t)(S)->csr[ 2] << 16))
> +#define CSR_CRBA(S) ((S)->csr[18] | ((uint32_t)(S)->csr[19] << 16))
> +#define CSR_CXBA(S) ((S)->csr[20] | ((uint32_t)(S)->csr[21] << 16))
> +#define CSR_NRBA(S) ((S)->csr[22] | ((uint32_t)(S)->csr[23] << 16))
> +#define CSR_BADR(S) ((S)->csr[24] | ((uint32_t)(S)->csr[25] << 16))
> +#define CSR_NRDA(S) ((S)->csr[26] | ((uint32_t)(S)->csr[27] << 16))
> +#define CSR_CRDA(S) ((S)->csr[28] | ((uint32_t)(S)->csr[29] << 16))
> +#define CSR_BADX(S) ((S)->csr[30] | ((uint32_t)(S)->csr[31] << 16))
> +#define CSR_NXDA(S) ((S)->csr[32] | ((uint32_t)(S)->csr[33] << 16))
> +#define CSR_CXDA(S) ((S)->csr[34] | ((uint32_t)(S)->csr[35] << 16))
> +#define CSR_NNRD(S) ((S)->csr[36] | ((uint32_t)(S)->csr[37] << 16))
> +#define CSR_NNXD(S) ((S)->csr[38] | ((uint32_t)(S)->csr[39] << 16))
> +#define CSR_PXDA(S) ((S)->csr[60] | ((uint32_t)(S)->csr[61] << 16))
> +#define CSR_NXBA(S) ((S)->csr[64] | ((uint32_t)(S)->csr[65] << 16))
>
> #define PHYSADDR(S,A) \
> - (BCR_SSIZE32(S) ? (A) : (A) | ((0xff00 & (uint32_t)(s)->csr[2])<<16))
> + (BCR_SSIZE32(S) ? (A) : (A) | ((0xff00 & (uint32_t)(S)->csr[2])<<16))
>
> struct pcnet_initblk16 {
> uint16_t mode;
>
>
>
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-04-01 20:35 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-15 16:47 [Qemu-devel] [PATCH] pcnet: Fix sign extension: make ipxe work with >2G RAM Alex Williamson
2011-03-17 9:08 ` [Qemu-devel] " Stefan Hajnoczi
2011-04-01 20:35 ` [Qemu-devel] " Aurelien Jarno
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).