[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH target-arm v1 7/7] dma/pl330: implement dmaadnh
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH target-arm v1 7/7] dma/pl330: implement dmaadnh instruction |
Date: |
Wed, 12 Feb 2014 19:15:37 +0000 |
On 11 February 2014 06:32, Peter Crosthwaite
<address@hidden> wrote:
> Implement the missing DMAADNH instruction. This is a minor variant
> of the DMAADDH instruction, so factor out to a common implementation
> for both (dmaadxh).
>
> Signed-off-by: Peter Crosthwaite <address@hidden>
> ---
>
> hw/dma/pl330.c | 19 ++++++++++++++++---
> 1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/hw/dma/pl330.c b/hw/dma/pl330.c
> index fe437cb..7acb2c4 100644
> --- a/hw/dma/pl330.c
> +++ b/hw/dma/pl330.c
> @@ -601,10 +601,12 @@ static inline void pl330_fault(PL330Chan *ch, uint32_t
> flags)
> * LEN - number of elements in ARGS array
> */
>
> -static void pl330_dmaaddh(PL330Chan *ch, uint8_t opcode, uint8_t *args, int
> len)
> +static void pl330_dmaadxh(PL330Chan *ch, uint8_t *args, bool ra, bool neg)
> {
> - uint16_t im = (((uint16_t)args[1]) << 8) | ((uint16_t)args[0]);
> - uint8_t ra = (opcode >> 1) & 1;
> + uint32_t im = (((uint16_t)args[1]) << 8) | ((uint16_t)args[0]);
These casts look kind of weird now that you've changed the type of 'im'.
In fact, you don't need them at all since the usual integer promotions
will take them up to 'int' which we know is 32 bits for us, and we're
not trying to shift into bit 31...
> + if (neg) {
> + im |= 0xffff << 16;
> + }
...speaking of which, you want a 'U' suffix on the constant, otherwise
you're doing a signed shift into the sign bit, which is undefined behaviour.
I checked the docs, and this is the correct behaviour -- we're one-extending
the immediate, not sign-extending it. Maybe this could use a brief comment:
/* One-extend the unsigned 16 bit value to 32 bits */
thanks
-- PMM
- [Qemu-devel] [PATCH target-arm v1 0/7] PL330 Changes, Peter Crosthwaite, 2014/02/11
- [Qemu-devel] [PATCH target-arm v1 1/7] dma/pl330: Delete overly verbose debug printf, Peter Crosthwaite, 2014/02/11
- [Qemu-devel] [PATCH target-arm v1 2/7] dma/pl330: Fix misleading type, Peter Crosthwaite, 2014/02/11
- [Qemu-devel] [PATCH target-arm v1 3/7] dma/pl330: printf format type sweep., Peter Crosthwaite, 2014/02/11
- [Qemu-devel] [PATCH target-arm v1 4/7] dma/pl330: Rename parent_obj, Peter Crosthwaite, 2014/02/11
- [Qemu-devel] [PATCH target-arm v1 5/7] dma/pl330: Add event debugging printfs, Peter Crosthwaite, 2014/02/11
- [Qemu-devel] [PATCH target-arm v1 6/7] dma/pl330: Fix buffer depth, Peter Crosthwaite, 2014/02/11
- [Qemu-devel] [PATCH target-arm v1 7/7] dma/pl330: implement dmaadnh instruction, Peter Crosthwaite, 2014/02/11
- Re: [Qemu-devel] [PATCH target-arm v1 7/7] dma/pl330: implement dmaadnh instruction,
Peter Maydell <=
- Re: [Qemu-devel] [PATCH target-arm v1 0/7] PL330 Changes, Peter Maydell, 2014/02/12