From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:58113)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <tamlokveer@gmail.com>) id 1XA5rA-0006YG-3J
	for qemu-devel@nongnu.org; Wed, 23 Jul 2014 19:21:57 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <tamlokveer@gmail.com>) id 1XA5r9-0008NG-3k
	for qemu-devel@nongnu.org; Wed, 23 Jul 2014 19:21:56 -0400
Received: from mail-qa0-x233.google.com ([2607:f8b0:400d:c00::233]:43525)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <tamlokveer@gmail.com>) id 1XA5r9-0008Lg-00
	for qemu-devel@nongnu.org; Wed, 23 Jul 2014 19:21:55 -0400
Received: by mail-qa0-f51.google.com with SMTP id k15so2027134qaq.38
	for <qemu-devel@nongnu.org>; Wed, 23 Jul 2014 16:21:54 -0700 (PDT)
MIME-Version: 1.0
In-Reply-To: <53CF6B12.5070700@redhat.com>
References: <1406044070-25667-1-git-send-email-tamlokveer@gmail.com>
	<1406044070-25667-2-git-send-email-tamlokveer@gmail.com>
	<53CF6B12.5070700@redhat.com>
Date: Thu, 24 Jul 2014 07:21:54 +0800
Message-ID: <CACrq+iKF2gmx+9+uj+k88T_ReJHeWQKn99QRJXJNDckpVqZ_fA@mail.gmail.com>
From: Le Tan <tamlokveer@gmail.com>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [Qemu-devel] [PATCH 1/3] intel-iommu: introduce Intel IOMMU
	(VT-d) emulation
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>, Knut Omang <knut.omang@oracle.com>, qemu-devel <qemu-devel@nongnu.org>, Alex Williamson <alex.williamson@redhat.com>, Jan Kiszka <jan.kiszka@web.de>, Anthony Liguori <aliguori@amazon.com>

Hi Paolo,

2014-07-23 15:58 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
> Il 22/07/2014 17:47, Le Tan ha scritto:
>> +static inline void define_quad(IntelIOMMUState *s, hwaddr addr, uint64_t val,
>> +                        uint64_t wmask, uint64_t w1cmask)
>> +{
>> +    *((uint64_t *)&s->csr[addr]) = val;
>
> All these casts are not endian-safe.  Please use ldl_le_p, ldq_le_p,
> stl_le_p, stq_le_p.

Thanks very much. Finally I got the idea here.:) Also thanks for your
renaming suggestions.

>> +    *((uint64_t *)&s->wmask[addr]) = wmask;
>> +    *((uint64_t *)&s->w1cmask[addr]) = w1cmask;
>> +}
>> +
>> +static inline void define_quad_wo(IntelIOMMUState *s, hwaddr addr,
>> +                                  uint64_t mask)
>> +{
>> +    *((uint64_t *)&s->womask[addr]) = mask;
>> +}
>> +
>> +static inline void define_long(IntelIOMMUState *s, hwaddr addr, uint32_t val,
>> +                        uint32_t wmask, uint32_t w1cmask)
>> +{
>> +    *((uint32_t *)&s->csr[addr]) = val;
>> +    *((uint32_t *)&s->wmask[addr]) = wmask;
>> +    *((uint32_t *)&s->w1cmask[addr]) = w1cmask;
>> +}
>> +
>> +static inline void define_long_wo(IntelIOMMUState *s, hwaddr addr,
>> +                                  uint32_t mask)
>> +{
>> +    *((uint32_t *)&s->womask[addr]) = mask;
>> +}
>> +
>> +/* "External" get/set operations */
>> +static inline void set_quad(IntelIOMMUState *s, hwaddr addr, uint64_t val)
>> +{
>> +    uint64_t oldval = *((uint64_t *)&s->csr[addr]);
>> +    uint64_t wmask = *((uint64_t *)&s->wmask[addr]);
>> +    uint64_t w1cmask = *((uint64_t *)&s->w1cmask[addr]);
>> +    *((uint64_t *)&s->csr[addr]) =
>> +        ((oldval & ~wmask) | (val & wmask)) & ~(w1cmask & val);
>> +}
>> +
>> +static inline void set_long(IntelIOMMUState *s, hwaddr addr, uint32_t val)
>> +{
>> +    uint32_t oldval = *((uint32_t *)&s->csr[addr]);
>> +    uint32_t wmask = *((uint32_t *)&s->wmask[addr]);
>> +    uint32_t w1cmask = *((uint32_t *)&s->w1cmask[addr]);
>> +    *((uint32_t *)&s->csr[addr]) =
>> +        ((oldval & ~wmask) | (val & wmask)) & ~(w1cmask & val);
>> +}
>> +
>> +static inline uint64_t get_quad(IntelIOMMUState *s, hwaddr addr)
>> +{
>> +    uint64_t val = *((uint64_t *)&s->csr[addr]);
>> +    uint64_t womask = *((uint64_t *)&s->womask[addr]);
>> +    return val & ~womask;
>> +}
>> +
>> +
>> +static inline uint32_t get_long(IntelIOMMUState *s, hwaddr addr)
>> +{
>> +    uint32_t val = *((uint32_t *)&s->csr[addr]);
>> +    uint32_t womask = *((uint32_t *)&s->womask[addr]);
>> +    return val & ~womask;
>> +}
>> +
>> +
>> +
>> +/* "Internal" get/set operations */
>> +static inline uint64_t __get_quad(IntelIOMMUState *s, hwaddr addr)
>
> get_quad_raw?
>
>> +{
>> +    return *((uint64_t *)&s->csr[addr]);
>> +}
>> +
>> +static inline uint32_t __get_long(IntelIOMMUState *s, hwaddr addr)
>
> get_long_raw?
>
>> +{
>> +    return *((uint32_t *)&s->csr[addr]);
>> +}
>> +
>> +
>> +/* val = (val & ~clear) | mask */
>> +static inline uint32_t set_mask_long(IntelIOMMUState *s, hwaddr addr,
>
> set_clear_long?
>
>> +                                     uint32_t clear, uint32_t mask)
>> +{
>> +    uint32_t *ptr = (uint32_t *)&s->csr[addr];
>> +    uint32_t val = (*ptr & ~clear) | mask;
>> +    *ptr = val;
>> +    return val;
>> +}
>> +
>> +/* val = (val & ~clear) | mask */
>> +static inline uint64_t set_mask_quad(IntelIOMMUState *s, hwaddr addr,
>
> set_clear_quad?
>> +                                     uint64_t clear, uint64_t mask)
>> +{
>> +    uint64_t *ptr = (uint64_t *)&s->csr[addr];
>> +    uint64_t val = (*ptr & ~clear) | mask;
>> +    *ptr = val;
>> +    return val;
>> +}
>> +
>> +
>

Regards,
Le