From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tamas K Lengyel Subject: Re: [PATCH] vm_event: Implement ARM SMC events Date: Tue, 12 Apr 2016 09:05:54 -0600 Message-ID: References: <1460404042-31179-1-git-send-email-tamas@tklengyel.com> <570C881A02000078000E6497@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============3375742187569890949==" Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1apzt7-0007Nl-OF for xen-devel@lists.xenproject.org; Tue, 12 Apr 2016 15:05:57 +0000 Received: by mail-yw0-f173.google.com with SMTP id t10so27819915ywa.0 for ; Tue, 12 Apr 2016 08:05:55 -0700 (PDT) In-Reply-To: <570C881A02000078000E6497@prv-mh.provo.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: Jan Beulich Cc: wei.liu2@citrix.com, Keir Fraser , Razvan Cojocaru , stefano.stabellini@eu.citrix.com, andrew.cooper3@citrix.com, ian.jackson@eu.citrix.com, julien.grall@arm.com, xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org --===============3375742187569890949== Content-Type: multipart/alternative; boundary=001a1149041a6e683005304aff34 --001a1149041a6e683005304aff34 Content-Type: text/plain; charset=UTF-8 On Apr 11, 2016 22:31, "Jan Beulich" wrote: > > >>> Tamas K Lengyel 04/11/16 9:47 PM >>> > >--- a/xen/include/public/vm_event.h > >+++ b/xen/include/public/vm_event.h > >@@ -166,6 +166,31 @@ struct vm_event_regs_x86 { > >uint32_t _pad; > >}; > > > >+struct vm_event_regs_arm { > >+ uint32_t r0; > >+ uint32_t r1; > >+ uint32_t r2; > >+ uint32_t r3; > >+ uint32_t r4; > >+ uint32_t r5; > >+ uint32_t r6; > >+ uint32_t r7; > >+ uint32_t r8; > >+ uint32_t r9; > >+ uint32_t r10; > >+ uint32_t r11; > >+ uint32_t r12; > >+ > >+ uint32_t sp; /* r13 - SP: Valid for Hyp. frames only, o/w banked (see below) */ > >+ uint32_t lr; /* r14 - LR: Valid for Hyp. Same physical register as lr_usr. */ > >+ > >+ uint32_t cpsr; /* Return mode */ > >+ uint64_t pc; > > Why uint64_t instead of uint32_t? No apparent reason, will fix. > > >+ uint64_t ttbr0; > >+ uint64_t ttbr1; > >+ uint32_t _pad; > >+}; > > This padding field is pretty strange: Without the adjustment to pc there are 16 > 32-bit fields (not counting the padding one), so I don't see why padding would be > needed at all. And with pc adjusted the padding field would surely better go > ahead of the two remaining 64-bit ones. Yes, I have been shuffling this struct around and didn't check the layout. Will fix. I'll also try to make this struct usable for aarch64 too. Tamas --001a1149041a6e683005304aff34 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable


On Apr 11, 2016 22:31, "Jan Beulich" <jbeulich@suse.com> wrote:
>
> >>> Tamas K Lengyel <tamas@tklengyel.com> 04/11/16 9:47 PM >>>
> >--- a/xen/include/public/vm_event.h
> >+++ b/xen/include/public/vm_event.h
> >@@ -166,6 +166,31 @@ struct vm_event_regs_x86 {
> =C2=A0 =C2=A0 =C2=A0>uint32_t _pad;
> =C2=A0>};
> =C2=A0>
> >+struct vm_event_regs_arm {
> >+=C2=A0 =C2=A0 uint32_t r0;
> >+=C2=A0 =C2=A0 uint32_t r1;
> >+=C2=A0 =C2=A0 uint32_t r2;
> >+=C2=A0 =C2=A0 uint32_t r3;
> >+=C2=A0 =C2=A0 uint32_t r4;
> >+=C2=A0 =C2=A0 uint32_t r5;
> >+=C2=A0 =C2=A0 uint32_t r6;
> >+=C2=A0 =C2=A0 uint32_t r7;
> >+=C2=A0 =C2=A0 uint32_t r8;
> >+=C2=A0 =C2=A0 uint32_t r9;
> >+=C2=A0 =C2=A0 uint32_t r10;
> >+=C2=A0 =C2=A0 uint32_t r11;
> >+=C2=A0 =C2=A0 uint32_t r12;
> >+
> >+=C2=A0 =C2=A0 uint32_t sp; /* r13 - SP: Valid for Hyp. frames onl= y, o/w banked (see below) */
> >+=C2=A0 =C2=A0 uint32_t lr; /* r14 - LR: Valid for Hyp. Same physi= cal register as lr_usr. */
> >+
> >+=C2=A0 =C2=A0 uint32_t cpsr; /* Return mode */
> >+=C2=A0 =C2=A0 uint64_t pc;
>
> Why uint64_t instead of uint32_t?

No apparent reason, will fix.

>
> >+=C2=A0 =C2=A0 uint64_t ttbr0;
> >+=C2=A0 =C2=A0 uint64_t ttbr1;
> >+=C2=A0 =C2=A0 uint32_t _pad;
> >+};
>
> This padding field is pretty strange: Without the adjustment to pc the= re are 16
> 32-bit fields (not counting the padding one), so I don't see why p= adding would be
> needed at all. And with pc adjusted the padding field would surely bet= ter go
> ahead of the two remaining 64-bit ones.

Yes, I have been shuffling this struct around and didn't= check the layout. Will fix. I'll also try to make this struct usable f= or aarch64 too.

Tamas

--001a1149041a6e683005304aff34-- --===============3375742187569890949== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwOi8vbGlzdHMueGVuLm9y Zy94ZW4tZGV2ZWwK --===============3375742187569890949==--