All of lore.kernel.org
 help / color / mirror / Atom feed
From: Scott Wood <scottwood@freescale.com>
To: Alexander Graf <agraf@suse.de>
Cc: kvm-ppc@vger.kernel.org, kvm list <kvm@vger.kernel.org>,
	Avi Kivity <avi@redhat.com>,
	Marcelo Tosatti <mtosatti@redhat.com>
Subject: Re: [PATCH] KVM: PPC: Add generic single register ioctls
Date: Fri, 06 Jan 2012 19:32:21 +0000	[thread overview]
Message-ID: <4F074C45.5000507@freescale.com> (raw)
In-Reply-To: <1325823314-12758-1-git-send-email-agraf@suse.de>

On 01/05/2012 10:15 PM, Alexander Graf wrote:
> Right now we transfer a static struct every time we want to get or set
> registers. Unfortunately, over time we realize that there are more of
> these than we thought of before and the extensibility and flexibility of
> transferring a full struct every time is limited.
> 
> So this is a new approach to the problem. With these new ioctls, we can
> get and set a single register that is identified by an ID. This allows for
> very precise and limited transmittal of data. When we later realize that
> it's a better idea to shove over multiple registers at once, we can reuse
> most of the infrastructure and simply implement a GET_MANY_REGS / SET_MANY_REGS
> interface.
> 
> The only downpoint I see to this one is that it needs to pad to 1024 bits
> (hardware is already on 512 bit registers, so I wanted to leave some room)
> which is slightly too much for transmitting only 64 bits. But if that's all
> the tradeoff we have to do for getting an extensible interface, I'd say go
> for it nevertheless.

The comment about padding is no longer relevant.

> +/*
> + * Architecture specific registers are to be defined in arch headers and
> + * ORed with the arch identifier.
> + */
> +#define KVM_REG_PPC		0x1000000000000000ULL
> +#define KVM_REG_X86		0x2000000000000000ULL
> +#define KVM_REG_IA64		0x3000000000000000ULL
> +#define KVM_REG_ARM		0x4000000000000000ULL
> +#define KVM_REG_S390		0x5000000000000000ULL
> +
> +#define KVM_REG_SIZE_SHIFT	52
> +#define KVM_REG_SIZE_MASK	0x00f0000000000000ULL
> +#define KVM_REG_SIZE_U8		0x0000000000000000ULL
> +#define KVM_REG_SIZE_U16	0x0010000000000000ULL
> +#define KVM_REG_SIZE_U32	0x0020000000000000ULL
> +#define KVM_REG_SIZE_U64	0x0030000000000000ULL
> +#define KVM_REG_SIZE_U128	0x0040000000000000ULL
> +#define KVM_REG_SIZE_U256	0x0050000000000000ULL
> +#define KVM_REG_SIZE_U512	0x0060000000000000ULL
> +#define KVM_REG_SIZE_U1024	0x0070000000000000ULL

Why not just encode directly as number of bytes?

-Scott


WARNING: multiple messages have this Message-ID (diff)
From: Scott Wood <scottwood@freescale.com>
To: Alexander Graf <agraf@suse.de>
Cc: <kvm-ppc@vger.kernel.org>, kvm list <kvm@vger.kernel.org>,
	Avi Kivity <avi@redhat.com>,
	Marcelo Tosatti <mtosatti@redhat.com>
Subject: Re: [PATCH] KVM: PPC: Add generic single register ioctls
Date: Fri, 6 Jan 2012 13:32:21 -0600	[thread overview]
Message-ID: <4F074C45.5000507@freescale.com> (raw)
In-Reply-To: <1325823314-12758-1-git-send-email-agraf@suse.de>

On 01/05/2012 10:15 PM, Alexander Graf wrote:
> Right now we transfer a static struct every time we want to get or set
> registers. Unfortunately, over time we realize that there are more of
> these than we thought of before and the extensibility and flexibility of
> transferring a full struct every time is limited.
> 
> So this is a new approach to the problem. With these new ioctls, we can
> get and set a single register that is identified by an ID. This allows for
> very precise and limited transmittal of data. When we later realize that
> it's a better idea to shove over multiple registers at once, we can reuse
> most of the infrastructure and simply implement a GET_MANY_REGS / SET_MANY_REGS
> interface.
> 
> The only downpoint I see to this one is that it needs to pad to 1024 bits
> (hardware is already on 512 bit registers, so I wanted to leave some room)
> which is slightly too much for transmitting only 64 bits. But if that's all
> the tradeoff we have to do for getting an extensible interface, I'd say go
> for it nevertheless.

The comment about padding is no longer relevant.

> +/*
> + * Architecture specific registers are to be defined in arch headers and
> + * ORed with the arch identifier.
> + */
> +#define KVM_REG_PPC		0x1000000000000000ULL
> +#define KVM_REG_X86		0x2000000000000000ULL
> +#define KVM_REG_IA64		0x3000000000000000ULL
> +#define KVM_REG_ARM		0x4000000000000000ULL
> +#define KVM_REG_S390		0x5000000000000000ULL
> +
> +#define KVM_REG_SIZE_SHIFT	52
> +#define KVM_REG_SIZE_MASK	0x00f0000000000000ULL
> +#define KVM_REG_SIZE_U8		0x0000000000000000ULL
> +#define KVM_REG_SIZE_U16	0x0010000000000000ULL
> +#define KVM_REG_SIZE_U32	0x0020000000000000ULL
> +#define KVM_REG_SIZE_U64	0x0030000000000000ULL
> +#define KVM_REG_SIZE_U128	0x0040000000000000ULL
> +#define KVM_REG_SIZE_U256	0x0050000000000000ULL
> +#define KVM_REG_SIZE_U512	0x0060000000000000ULL
> +#define KVM_REG_SIZE_U1024	0x0070000000000000ULL

Why not just encode directly as number of bytes?

-Scott


  reply	other threads:[~2012-01-06 19:32 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-06  3:59 [PATCH 0/3] GET/SET_ONE_REG and HIOR patches v2 Alexander Graf
2012-01-06  3:59 ` Alexander Graf
2012-01-06  3:59 ` [PATCH 1/3] KVM: PPC: Add generic single register ioctls Alexander Graf
2012-01-06  3:59   ` Alexander Graf
2012-01-06  3:49   ` Alexander Graf
2012-01-06  3:49     ` Alexander Graf
2012-01-06  4:15     ` [PATCH] " Alexander Graf
2012-01-06  4:15       ` Alexander Graf
2012-01-06 19:32       ` Scott Wood [this message]
2012-01-06 19:32         ` Scott Wood
2012-01-07  0:52         ` Alexander Graf
2012-01-07  0:52           ` Alexander Graf
2012-01-09 19:07           ` Scott Wood
2012-01-09 19:07             ` Scott Wood
2012-01-09 20:11             ` Alexander Graf
2012-01-09 20:11               ` Alexander Graf
2012-01-09 20:12               ` Scott Wood
2012-01-09 20:12                 ` Scott Wood
2012-01-09 20:14                 ` Alexander Graf
2012-01-09 20:14                   ` Alexander Graf
2012-01-06  3:59 ` [PATCH 2/3] KVM: PPC: Add support for explicit HIOR setting Alexander Graf
2012-01-06  3:59   ` Alexander Graf
2012-01-06 18:42   ` Scott Wood
2012-01-06 18:42     ` Scott Wood
2012-04-01 19:51   ` Andreas Schwab
2012-04-01 19:51     ` Andreas Schwab
2012-04-16 16:54     ` Alexander Graf
2012-04-16 16:54       ` Alexander Graf
2012-04-16 21:42       ` Paul Mackerras
2012-04-16 21:42         ` Paul Mackerras
2012-01-06  3:59 ` [PATCH 3/3] KVM: PPC: Move kvm_vcpu_ioctl_[gs]et_one_reg down to platform-specific code Alexander Graf
2012-01-06  3:59   ` Alexander Graf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4F074C45.5000507@freescale.com \
    --to=scottwood@freescale.com \
    --cc=agraf@suse.de \
    --cc=avi@redhat.com \
    --cc=kvm-ppc@vger.kernel.org \
    --cc=kvm@vger.kernel.org \
    --cc=mtosatti@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.