Hello Heinz,
i'm traveling and will be back in office next week Monday, we'll review the offer then and should respond soon.

BR,
Mario

----- Reply message -----
From: "Pranavkumar Sawargaonkar" <pranavkumar@linaro.org>
To: "Christopher Covington" <cov@codeaurora.org>
Cc: "linaro-kernel@lists.linaro.org" <linaro-kernel@lists.linaro.org>, "kvmarm@lists.cs.columbia.edu" <kvmarm@lists.cs.columbia.edu>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>, "Patch Tracking" <patches@linaro.org>
Subject: [PATCH V2] arm64: KVM: Fix HCR_EL2 and VTCR_EL2 configuration bits
Date: Tue, May 14, 2013 08:53



Hi Christopher,

On 13 May 2013 22:46, Christopher Covington <cov@codeaurora.org> wrote:
> On 05/13/2013 12:02 PM, Marc Zyngier wrote:
>> On Mon, 13 May 2013 11:23:05 -0400, Christopher Covington
>> <cov@codeaurora.org> wrote:
>>> Hi Pranavkumar,
>>>
>>> On 05/10/2013 03:23 AM, Pranavkumar Sawargaonkar wrote:
>>>> This patch does following fixes:
>>>>
>>>> 1. Make HCR_* flags as unsigned long long constants
>>>> Reason : By default, compiler assumes numeric constants as
>>>> signed hence sign extending it when assigned to unsigned variable
>>>> such as hcr_el2 (in VCPU context). This accidently sets HCR_ID and
>>>> HCR_CD making entire guest memory non-cacheable. On real HW, this
>>>> breaks Stage2 ttbl walks and also breaks VirtIO.
>>>>
>>>> 2. VTCR_EL2_ORGN0_WBWA and VTCR_EL2_IRGN0_WBWA macros.
>>>>
>>>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>>>> Signed-off-by: Anup Patel <anup.patel@linaro.org>
>>>> ---
>>>>  arch/arm64/include/asm/kvm_arm.h |   73
>>>>  +++++++++++++++++++-------------------
>>>>  1 file changed, 37 insertions(+), 36 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/include/asm/kvm_arm.h
>>>> b/arch/arm64/include/asm/kvm_arm.h
>>>> index 8ced0ca..14ead69 100644
>>>> --- a/arch/arm64/include/asm/kvm_arm.h
>>>> +++ b/arch/arm64/include/asm/kvm_arm.h
>>>> @@ -18,44 +18,45 @@
>>>>  #ifndef __ARM64_KVM_ARM_H__
>>>>  #define __ARM64_KVM_ARM_H__
>>>>
>>>> +#include <linux/const.h>
>>>>  #include <asm/types.h>
>>>>
>>>>  /* Hyp Configuration Register (HCR) bits */
>>>> -#define HCR_ID             (1 << 33)
>>>> -#define HCR_CD             (1 << 32)
>>>> +#define HCR_ID             (UL(0x1) << 33)
>>>> +#define HCR_CD             (UL(0x1) << 32)
>>>
>>> Consider using the BIT() macro from linux/bitops.h to improve
>> readability.
>>
>> This would probably cause problems when using these macros from assembly
>> code.
>
> It does indeed cause problems. What a shame. How about a quick local
> definition, to perhaps be eventually replaced by a more accessible shared
> definition?

I have used UL to be synced with current arm64 code style (it has been
used in other files also).
Using BIT is good idea (not sure about local definition i.e. how good
it is when we have one inside bitops.h)
but if we want to use it is, then better idea is to use it everywhere.
>
> Christopher

Thanks,
Pranav

>
> --
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> hosted by the Linux Foundation.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm