On Wednesday 24 December 2008 01:55:42 Marcelo Tosatti wrote:
> On Tue, Dec 23, 2008 at 04:00:27PM +0800, Sheng Yang wrote:
> > Avi's purpose, to use single kvm_set_irq() to deal with all interrupt,
> > including MSI. So here is it.
> >
> > struct gsi_msg is a mapping from a special gsi(with KVM_GSI_MSG_MASK) to
> > MSI/MSI-X message address/data.
> >
> > Now we support up to 256 gsi_msg mapping, and gsi_msg is allocated by
> > kernel and provide two ioctls to userspace, which is more flexiable.
> >
> > Signed-off-by: Sheng Yang <[email protected]>
> > ---
> > include/linux/kvm.h | 12 ++++++++
> > include/linux/kvm_host.h | 16 ++++++++++
> > virt/kvm/irq_comm.c | 70
> > ++++++++++++++++++++++++++++++++++++++++++++++ virt/kvm/kvm_main.c |
> > 63 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 161
> > insertions(+), 0 deletions(-)
> >
> > diff --git a/include/linux/kvm.h b/include/linux/kvm.h
> > index 5b965f6..b091a86 100644
> > --- a/include/linux/kvm.h
> > +++ b/include/linux/kvm.h
> > @@ -394,6 +394,7 @@ struct kvm_trace_rec {
> > #define KVM_CAP_USER_NMI 22
> > #endif
> > #define KVM_CAP_SET_GUEST_DEBUG 23
> > +#define KVM_CAP_GSI_MSG 24
> >
> > /*
> > * ioctls for VM fds
> > @@ -427,6 +428,8 @@ struct kvm_trace_rec {
> > struct kvm_assigned_pci_dev)
> > #define KVM_ASSIGN_IRQ _IOR(KVMIO, 0x70, \
> > struct kvm_assigned_irq)
> > +#define KVM_REQUEST_GSI_MSG _IOWR(KVMIO, 0x71, struct
> > kvm_assigned_gsi_msg) +#define KVM_FREE_GSI_MSG _IOR(KVMIO, 0x72, __u32)
>
> Wrap the __u32 into a struct. Could use kvm_assigned_gsi_msg itself.
OK.
> > +++ b/include/linux/kvm_host.h
> > @@ -132,6 +132,10 @@ struct kvm {
> > unsigned long mmu_notifier_seq;
> > long mmu_notifier_count;
> > #endif
> > + struct hlist_head gsi_msg_list;
> > + struct mutex gsi_msg_lock;
> > +#define KVM_NR_GSI_MSG 256
> > + DECLARE_BITMAP(gsi_msg_bitmap, KVM_NR_GSI_MSG);
> > };
>
> This is platform specific data. Can't it live in kvm_arch?
No... It's not platform specific data, at least, we want it to use with x86,
IA64 and virtio, that's why I make it so generic...
>
> > }
> > +
> > +int kvm_update_gsi_msg(struct kvm *kvm, struct kvm_gsi_msg *gsi_msg)
> > +{
> > + struct kvm_gsi_msg *found_msg, *new_gsi_msg;
> > + int r, gsi;
> > +
> > + mutex_lock(&kvm->gsi_msg_lock);
> > + /* Find whether we need a update or a new entry */
> > + found_msg = kvm_find_gsi_msg(kvm, gsi_msg->gsi);
> > + if (found_msg)
> > + *found_msg = *gsi_msg;
> > + else {
> > + gsi = find_first_zero_bit(kvm->gsi_msg_bitmap, KVM_NR_GSI_MSG);
> > + if (gsi >= KVM_NR_GSI_MSG) {
> > + r = -EFAULT;
>
> ENOSPC?
OK. (Though I am confusing with all kinds of ERR all the time, ENOSPC show "No
space left in the device"... And last time somebody told me "ENOTTY" means
something is not available...)
>
> > +static int kvm_vm_ioctl_request_gsi_msg(struct kvm *kvm,
> > + struct kvm_assigned_gsi_msg *agsi_msg)
> > +{
> > + struct kvm_gsi_msg gsi_msg;
> > + int r;
> > +
> > + gsi_msg.gsi = agsi_msg->gsi;
> > + gsi_msg.msg.address_lo = agsi_msg->msg.addr_lo;
> > + gsi_msg.msg.address_hi = agsi_msg->msg.addr_hi;
> > + gsi_msg.msg.data = agsi_msg->msg.data;
> > +
> > + r = kvm_update_gsi_msg(kvm, &gsi_msg);
> > + if (r == 0)
> > + agsi_msg->gsi = gsi_msg.gsi;
> > + return r;
> > +}
>
> Can't see the purpose of this function. Why preserve the user-passed GSI
> value in case of failure? It will return an error anyway...
Yeah, for the MSI(due to we emulated the configuration space in the userspace)
and virtio.
Why it would return a error? I missed something?
--
regards
Yang, Sheng
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
