Messages in this thread | | | Date | Sun, 27 Sep 2020 12:44:14 +0200 | From | Greg Kroah-Hartman <> | Subject | Re: [PATCH v4 17/17] virt: acrn: Introduce an interface for Service VM to control vCPU |
| |
On Tue, Sep 22, 2020 at 07:43:11PM +0800, shuo.a.liu@intel.com wrote: > From: Shuo Liu <shuo.a.liu@intel.com> > > ACRN supports partition mode to achieve real-time requirements. In > partition mode, a CPU core can be dedicated to a vCPU of User VM. The > local APIC of the dedicated CPU core can be passthrough to the User VM. > The Service VM controls the assignment of the CPU cores. > > Introduce an interface for the Service VM to remove the control of CPU > core from hypervisor perspective so that the CPU core can be a dedicated > CPU core of User VM. > > Signed-off-by: Shuo Liu <shuo.a.liu@intel.com> > Reviewed-by: Zhi Wang <zhi.a.wang@intel.com> > Reviewed-by: Reinette Chatre <reinette.chatre@intel.com> > Cc: Zhi Wang <zhi.a.wang@intel.com> > Cc: Zhenyu Wang <zhenyuw@linux.intel.com> > Cc: Yu Wang <yu1.wang@intel.com> > Cc: Reinette Chatre <reinette.chatre@intel.com> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > --- > drivers/virt/acrn/hsm.c | 50 +++++++++++++++++++++++++++++++++++ > drivers/virt/acrn/hypercall.h | 14 ++++++++++ > 2 files changed, 64 insertions(+) > > diff --git a/drivers/virt/acrn/hsm.c b/drivers/virt/acrn/hsm.c > index aaf4e76d27b4..ef5f77a38d1f 100644 > --- a/drivers/virt/acrn/hsm.c > +++ b/drivers/virt/acrn/hsm.c > @@ -9,6 +9,7 @@ > * Yakui Zhao <yakui.zhao@intel.com> > */ > > +#include <linux/cpu.h> > #include <linux/io.h> > #include <linux/mm.h> > #include <linux/module.h> > @@ -354,6 +355,47 @@ struct miscdevice acrn_dev = { > .fops = &acrn_fops, > }; > > +static ssize_t remove_cpu_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + u64 cpu, lapicid; > + int ret; > + > + if (kstrtoull(buf, 0, &cpu) < 0) > + return -EINVAL; > + > + if (cpu >= num_possible_cpus() || cpu == 0 || !cpu_is_hotpluggable(cpu)) > + return -EINVAL; > + > + if (cpu_online(cpu)) > + remove_cpu(cpu); > + > + lapicid = cpu_data(cpu).apicid; > + dev_dbg(dev, "Try to remove cpu %lld with lapicid %lld\n", cpu, lapicid); > + ret = hcall_sos_remove_cpu(lapicid); > + if (ret < 0) { > + dev_err(dev, "Failed to remove cpu %lld!\n", cpu); > + goto fail_remove; > + } > + > + return count; > + > +fail_remove: > + add_cpu(cpu); > + return ret; > +} > +static DEVICE_ATTR_WO(remove_cpu); > + > +static struct attribute *acrn_attrs[] = { > + &dev_attr_remove_cpu.attr, > + NULL > +}; > + > +static struct attribute_group acrn_attr_group = { > + .attrs = acrn_attrs, > +};
You create a sysfs attribute without any Documentation/ABI/ update as well? That's not good.
And why are you trying to emulate CPU hotplug here and not using the existing CPU hotplug mechanism?
> + > static int __init hsm_init(void) > { > int ret; > @@ -370,13 +412,21 @@ static int __init hsm_init(void) > return ret; > } > > + ret = sysfs_create_group(&acrn_dev.this_device->kobj, &acrn_attr_group); > + if (ret) { > + dev_warn(acrn_dev.this_device, "sysfs create failed\n"); > + misc_deregister(&acrn_dev); > + return ret; > + }
You just raced with userspace and lost. If you want to add attribute files to a device, use the default attribute group list, and it will be managed properly for you by the driver core.
Huge hint, if a driver every has to touch a kobject, or call sysfs_*, then it is probably doing something wrong.
greg k-h
| |