Re: [PATCH] KVM: arm/arm64: Keep mapped, disabled interrupts quiet

2015-10-15 Thread Christoffer Dall
On Thu, Oct 08, 2015 at 01:36:09PM +0100, Marc Zyngier wrote:
> If a mapped interrupt is disabled, we must make sure the
> corresponding physical interrupt cannot fire, as we are not
> injecting the interrupt, and not setting the active bit.
> 
> For example, a guest disabling its timer interrupt at the GIC level
> but leaving the timer firing would stop making progress as noone
> would be able to prevent this timer from firing and interrupting
> the host. Not quite what is expected. And if we're rebooting
> or turning a vcpu off while the interrupt is about to fire,
> we're exactly going to face this.
> 
> In order to cope with this, parse the list of mapped interrupts,
> and mark it as active if we're about to run the guest (or inactive
> if we've exited). Hopefully, nobody is going to run with zillions
> of disabled, mapped interrupts, right?
> 
> Reported-by: Lorenzo Pieralisi 
> Signed-off-by: Marc Zyngier 
> ---
> My gut feeling is that this vgic_dist_irq_set_pending should always
> be done, but I'd like some other eyes to have a look at it.
> 
> Tested on Seattle (running 4.3-rc4) with a script hammering CPU hotplug.
> 
>  virt/kvm/arm/vgic.c | 60 
> +
>  1 file changed, 60 insertions(+)
> 
> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
> index 6bd1c9b..0ad3f7e 100644
> --- a/virt/kvm/arm/vgic.c
> +++ b/virt/kvm/arm/vgic.c
> @@ -1092,6 +1092,11 @@ static void vgic_retire_lr(int lr_nr, int irq, struct 
> kvm_vcpu *vcpu)
>   struct vgic_cpu *vgic_cpu = >arch.vgic_cpu;
>   struct vgic_lr vlr = vgic_get_lr(vcpu, lr_nr);
>  
> + if (vlr.state & LR_HW) {
> + vgic_dist_irq_set_pending(vcpu, irq);
> + vlr.hwirq = 0;
> + }
> +

I don't understand this logic?  At least you should check if the LR
state is active or pending, sort of what vgic_unqueue_irqs does.

In fact, it looks like we can re-use the vgic_unqueue_irqs if we do
some refactoring of that function.

But why is it necessary?

>   vlr.state = 0;
>   vgic_set_lr(vcpu, lr_nr, vlr);
>   clear_bit(lr_nr, vgic_cpu->lr_used);
> @@ -1232,6 +1237,57 @@ static bool vgic_queue_hwirq(struct kvm_vcpu *vcpu, 
> int irq)
>  }
>  
>  /*
> + * If a mapped interrupt is disabled, we must make sure the
> + * corresponding physical interrupt cannot fire, as we are not
> + * injecting the interrupt, and not setting the active bit.
> + *
> + * Parse the list of mapped interrupts, and mark it as active if we're
> + * about to run the guest (or inactive if we've exited). Hopefully,
> + * nobody is going to run with zillions of disabled, mapped
> + * interrupts...
> + */
> +static void vgic_handle_disabled_mapped_irq(struct kvm_vcpu *vcpu, bool 
> enter)
> +{
> + struct vgic_dist *dist = >kvm->arch.vgic;
> + struct vgic_cpu *vgic_cpu = >arch.vgic_cpu;
> + struct vgic_bitmap *spi_bitmap;
> + struct list_head *root;
> + struct irq_phys_map_entry *entry;
> + struct irq_phys_map *map;
> + int ret;
> +
> + rcu_read_lock();
> +
> + /* Check for PPIs */
> + root = _cpu->irq_phys_map_list;
> + list_for_each_entry_rcu(entry, root, entry) {
> + map = >map;
> + if (!vgic_irq_is_enabled(vcpu, map->virt_irq)) {
> + ret = irq_set_irqchip_state(map->irq,
> + IRQCHIP_STATE_ACTIVE,
> + enter);
> + WARN_ON(ret);
> + }
> + }
> +
> + /* Check for SPIs routed to this vcpu */
> + root = >irq_phys_map_list;
> + spi_bitmap = >irq_spi_target[vcpu->vcpu_id];
> + list_for_each_entry_rcu(entry, root, entry) {
> + map = >map;
> + if (!vgic_irq_is_enabled(vcpu, map->virt_irq) &&
> + vgic_bitmap_get_irq_val(spi_bitmap, 0, map->virt_irq)) {

why do we check the emulated target bit here?

is the right thing not to see if we're running on the physical CPU which
the physical distributor is configured to route this SPI to, and in that
case set the IRQ active to prevent it from firting?

> + ret = irq_set_irqchip_state(map->irq,
> + IRQCHIP_STATE_ACTIVE,
> + enter);
> + WARN_ON(ret);
> + }
> + }
> +
> + rcu_read_unlock();
> +}
> +
> +/*
>   * Fill the list registers with pending interrupts before running the
>   * guest.
>   */
> @@ -1320,6 +1376,8 @@ epilog:
>   WARN_ON(ret);
>   }
>   }
> +
> + vgic_handle_disabled_mapped_irq(vcpu, true);
>  }
>  
>  static bool vgic_process_maintenance(struct kvm_vcpu *vcpu)
> @@ -1484,6 +1542,8 @@ static void __kvm_vgic_sync_hwstate(struct kvm_vcpu 
> *vcpu)
>   vgic_cpu->vgic_irq_lr_map[vlr.irq] = LR_EMPTY;
>   }
>  
> + 

Re: [PATCH] VFIO: platform: AMD xgbe reset module

2015-10-15 Thread Arnd Bergmann
On Thursday 15 October 2015 10:08:02 Eric Auger wrote:
> Hi Arnd,
> On 10/14/2015 05:38 PM, Arnd Bergmann wrote:
> > On Wednesday 14 October 2015 15:33:12 Eric Auger wrote:
> >> --- a/drivers/vfio/platform/vfio_platform_common.c
> >> +++ b/drivers/vfio/platform/vfio_platform_common.c
> >> @@ -31,6 +31,11 @@ static const struct vfio_platform_reset_combo 
> >> reset_lookup_table[] = {
> >> .reset_function_name = "vfio_platform_calxedaxgmac_reset",
> >> .module_name = "vfio-platform-calxedaxgmac",
> >> },
> >> +   {
> >> +   .compat = "amd,xgbe-seattle-v1a",
> >> +   .reset_function_name = "vfio_platform_amdxgbe_reset",
> >> +   .module_name = "vfio-platform-amdxgbe",
> >> +   },
> >>  };
> >>  
> >>  static void vfio_platform_get_reset(struct vfio_platform_device *vdev,
> >>
> > 
> > This is causing build errors for me when CONFIG_MODULES is disabled.
> Sorry about that and thanks for reporting the issue
> > 
> > Could this please be restructured so vfio_platform_get_reset does
> > not attempt to call __symbol_get() but instead has the drivers
> > register themselves properly to a subsystem?
> OK
> 
> Could you elaborate about "has the drivers register themselves properly
> to a subsystem".
> 
> My first proposal when coping with this problematic of being able to add
> reset plugins to the vfio-platform driver was to create new drivers per
> device requiring reset. but this was considered painful for end-users,
> who needed to be aware of the right driver to bind - and I think that
> makes sense - (https://lkml.org/lkml/2015/4/17/568) .

Having multiple drivers indeed sucks, but your current approach isn't
that much better, as you still have two modules that are used to driver
the same hardware.

I would expect that the same driver that is used for the normal
operation and that it calls a function to register itself to vfio
by passing a structure with the device and reset function pointer.

> A naive question I dare to ask, wouldn't it be acceptable to make
> vfio_platform depend on CONFIG_MODULES? Don't we disable modules for
> security purpose? In that context would we use VFIO?

I think a lot of embedded systems turn off modules to save a little
memory, speed up boot time and simplify their user space.

Aside from that, the current method is highly unusual and looks a bit
fragile to me, as you are relying on internals of the module loader
code. It's also a layering violation as the generic code needs to be
patched for each device specific module that is added, and we try
to avoid that.

A possible solution could be something inside the xgbe driver like


static void xgbe_init_module(void)
{
int ret = 0;

if (IS_ENABLED(CONFIG_AMD_XGBE_ETHERNET)
ret = platform_driver_register(_driver);
if (ret)
return ret;

if (IS_ENABLED(CONFIG_VFIO_PLATFORM))
ret = vfio_platform_register_reset(_of_match, 
xgbe_platform_reset);

return ret; 
}

This way you have exactly one driver module that gets loaded for the
device and you can use it either with the platform_driver or through
vfio.

A nicer way that would be a little more work would be to integrate
the reset infrastructure into 'struct platform_driver' framework,
by adding another callback to the it for doing the interaction with
vfio, something like

enum vfio_platform_op {
VFIO_PLATFORM_BIND,
VFIO_PLATFORM_UNBIND,
VFIO_PLATFORM_RESET,
};

struct platform_driver {
int (*probe)(struct platform_device *);
int (*remove)(struct platform_device *);
...
int (*vfio_manage)(struct platform_device *, enum vfio_platform_op);
struct device_driver driver;
};

This would integrate much more closely into the platform driver framework,
just like the regular vfio driver integrates into the PCI framework.
Unlike PCI however, you can't just use the generic driver framework to
unbind the driver, because you still need device specific code.

Arnd
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v3 0/4] KVM: arm/arm64: guest synchronous halt/resume

2015-10-15 Thread Christoffer Dall
On Fri, Sep 25, 2015 at 11:41:13PM +0200, Eric Auger wrote:
> This series introduces the capability to synchronously exit the guest
> and prevent it from being re-entered. This modality will be used by
> IRQ forwarding series when changing the state of the IRQ.
> 
> Former pause flag used when starting the vcpu in KVM_ARM_VCPU_POWER_OFF
> state, in PSCI calls and in KVM_SET_MP_STATE ioctl is renamed into
> power_off. A new pause flag is introduced. Both now are checked in
> kvm_arch_vcpu_runnable and in the VCPU_RUN critical section, before
> entering the vcpu.
> 
Thanks - applied,
-Christoffer
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH] KVM: arm: use GIC support unconditionally

2015-10-15 Thread Christoffer Dall
On Mon, Oct 12, 2015 at 03:22:31PM +0200, Arnd Bergmann wrote:
> The vgic code on ARM is built for all configurations that enable KVM,
> but the parent_data field that it references is only present when
> CONFIG_IRQ_DOMAIN_HIERARCHY is set:
> 
> virt/kvm/arm/vgic.c: In function 'kvm_vgic_map_phys_irq':
> virt/kvm/arm/vgic.c:1781:13: error: 'struct irq_data' has no member named 
> 'parent_data'
> 
> This flag is implied by the GIC driver, and indeed the VGIC code only
> makes sense if a GIC is present. This changes the CONFIG_KVM symbol
> to always select GIC, which avoids the issue.
> 
> Signed-off-by: Arnd Bergmann 
> Fixes: 662d9715840 ("arm/arm64: KVM: Kill CONFIG_KVM_ARM_{VGIC,TIMER}")
> 
> diff --git a/arch/arm/kvm/Kconfig b/arch/arm/kvm/Kconfig
> index 210eccadb69a..356970f3b25e 100644
> --- a/arch/arm/kvm/Kconfig
> +++ b/arch/arm/kvm/Kconfig
> @@ -21,6 +21,7 @@ config KVM
>   depends on MMU && OF
>   select PREEMPT_NOTIFIERS
>   select ANON_INODES
> + select ARM_GIC
>   select HAVE_KVM_CPU_RELAX_INTERCEPT
>   select HAVE_KVM_ARCH_TLB_FLUSH_ALL
>   select KVM_MMIO
> 

Applied.

Thanks,
-Christoffer
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH] VFIO: platform: AMD xgbe reset module

2015-10-15 Thread Eric Auger
Hi Arnd,
On 10/14/2015 05:38 PM, Arnd Bergmann wrote:
> On Wednesday 14 October 2015 15:33:12 Eric Auger wrote:
>> --- a/drivers/vfio/platform/vfio_platform_common.c
>> +++ b/drivers/vfio/platform/vfio_platform_common.c
>> @@ -31,6 +31,11 @@ static const struct vfio_platform_reset_combo 
>> reset_lookup_table[] = {
>> .reset_function_name = "vfio_platform_calxedaxgmac_reset",
>> .module_name = "vfio-platform-calxedaxgmac",
>> },
>> +   {
>> +   .compat = "amd,xgbe-seattle-v1a",
>> +   .reset_function_name = "vfio_platform_amdxgbe_reset",
>> +   .module_name = "vfio-platform-amdxgbe",
>> +   },
>>  };
>>  
>>  static void vfio_platform_get_reset(struct vfio_platform_device *vdev,
>>
> 
> This is causing build errors for me when CONFIG_MODULES is disabled.
Sorry about that and thanks for reporting the issue
> 
> Could this please be restructured so vfio_platform_get_reset does
> not attempt to call __symbol_get() but instead has the drivers
> register themselves properly to a subsystem?
OK

Could you elaborate about "has the drivers register themselves properly
to a subsystem".

My first proposal when coping with this problematic of being able to add
reset plugins to the vfio-platform driver was to create new drivers per
device requiring reset. but this was considered painful for end-users,
who needed to be aware of the right driver to bind - and I think that
makes sense - (https://lkml.org/lkml/2015/4/17/568) .

A naive question I dare to ask, wouldn't it be acceptable to make
vfio_platform depend on CONFIG_MODULES? Don't we disable modules for
security purpose? In that context would we use VFIO?

Best Regards

Eric
> 
> I don't see any way this could be fixed otherwise. The problem
> of course showed up with calxedaxgmac already, but I'd prefer not
> to see anything added there until the common code has been improved.
> 
>   Arnd
> 

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: its_alloc_tables() - with all BASER marked none

2015-10-15 Thread Marc Zyngier
On 15/10/15 02:30, Mario Smarduch wrote:
> If its_init()/its_probe()/its_alloc_tables() finds all GITS_BASER type none, 
> it
> continues with ITS initialization.
> 
> Is there a reason to continue?

Of course. There is nothing that *mandates* the ITS to request memory
from the operating system. The HW could perfectly come with its own
memory and not need anything at all.

> Started to look through this code recently - little confused here.

Only the beginning! ;-)

M.
-- 
Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH] VFIO: platform: AMD xgbe reset module

2015-10-15 Thread Arnd Bergmann
On Thursday 15 October 2015 16:20:46 Eric Auger wrote:
> On 10/15/2015 02:12 PM, Christoffer Dall wrote:
> > On Thu, Oct 15, 2015 at 01:21:55PM +0200, Arnd Bergmann wrote:
> >> On Thursday 15 October 2015 10:08:02 Eric Auger wrote:
> >>> On 10/14/2015 05:38 PM, Arnd Bergmann wrote:
>  On Wednesday 14 October 2015 15:33:12 Eric Auger wrote:
> >> A possible solution could be something inside the xgbe driver like
> >>
> >>
> >> static void xgbe_init_module(void)
> >> {
> >>int ret = 0;
> >>
> >>if (IS_ENABLED(CONFIG_AMD_XGBE_ETHERNET)
> >>ret = platform_driver_register(_driver);
> >>if (ret)
> >>return ret;
> >>
> >>if (IS_ENABLED(CONFIG_VFIO_PLATFORM))
> >>ret = vfio_platform_register_reset(_of_match, 
> >> xgbe_platform_reset);
> >>
> >>return ret; 
> >> }
> >>
> >> This way you have exactly one driver module that gets loaded for the
> >> device and you can use it either with the platform_driver or through
> >> vfio.
> If I understand it correctly you still need 2 loaded modules (VFIO
> driver & XGBE driver which implements the reset function) or am I
> missing something?

That is correct, yes.
 
> I had a similar mechanism of registration in my PATCH v1 but I did the
> registration from the reset module itself instead of in the native
> driver, as you suggest here.

Right. The main difference is that you don't have two modules fighting
over the same device with the approach here.

Arnd
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH] VFIO: platform: AMD xgbe reset module

2015-10-15 Thread Arnd Bergmann
On Thursday 15 October 2015 16:46:09 Eric Auger wrote:
> > 
> > This is where we'd need a little more changes for this approach. Instead
> > of unbinding the device from its driver, the idea would be that the
> > driver remains bound as far as the driver model is concerned, but
> > it would be in a quiescent state where no other subsystem interacts with
> > it (i.e. it gets unregistered from networking core or whichever it uses).
> 
> Currently we use the same mechanism as for PCI, ie. unbind the native
> driver and then bind VFIO platform driver in its place. Don't you think
> changing this may be a pain for user-space tools that are designed to
> work that way for PCI?
>
> My personal preference would be to start with your first proposal since
> it looks (to me) less complex and "unknown" that the 2d approach.

We certainly can't easily change from one approach to the other without
breaking user expectations, so the decision needs to be made carefully.

The main observation here is that platform devices are unlike PCI in this
regard because they need extra per-device code. I have argued in the
past that we should not reuse the "VFIO" name here because it's actually
something else.  On the other hand, there are a lot of commonalities,
we just have to make sure we don't try to force the code into one model
that doesn't really work just to make it look more like PCI VFIO.

Arnd
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH] VFIO: platform: AMD xgbe reset module

2015-10-15 Thread Christoffer Dall
On Thu, Oct 15, 2015 at 04:55:13PM +0200, Arnd Bergmann wrote:
> On Thursday 15 October 2015 16:46:09 Eric Auger wrote:
> > > 
> > > This is where we'd need a little more changes for this approach. Instead
> > > of unbinding the device from its driver, the idea would be that the
> > > driver remains bound as far as the driver model is concerned, but
> > > it would be in a quiescent state where no other subsystem interacts with
> > > it (i.e. it gets unregistered from networking core or whichever it uses).
> > 
> > Currently we use the same mechanism as for PCI, ie. unbind the native
> > driver and then bind VFIO platform driver in its place. Don't you think
> > changing this may be a pain for user-space tools that are designed to
> > work that way for PCI?
> >
> > My personal preference would be to start with your first proposal since
> > it looks (to me) less complex and "unknown" that the 2d approach.
> 
> We certainly can't easily change from one approach to the other without
> breaking user expectations, so the decision needs to be made carefully.
> 
> The main observation here is that platform devices are unlike PCI in this
> regard because they need extra per-device code. I have argued in the
> past that we should not reuse the "VFIO" name here because it's actually
> something else.  

I've adjusted to consider VFIO a general purpose framework for mapping
device resources into userspace/VMs, and there are certainly a lot of
commonality with both PCI, platform, and potentially other devices for
that to make sense.


> On the other hand, there are a lot of commonalities,
> we just have to make sure we don't try to force the code into one model
> that doesn't really work just to make it look more like PCI VFIO.
> 

But given that we now have code for platform device passthrough that
works in both QEMU and the kernel side and is actually useful for
people, is there a clear technical advantage to go back and rework thaat
at this point?

Don't get me wrong, I like the idea of having a single driver bound to a
platform device, and then that's it, but it just feels like that
discussion doesn't necessarily belong in the context of a patch that
'just' seeks to add reset functionality for a specific device for VFIO?

-Christoffer
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH] VFIO: platform: AMD xgbe reset module

2015-10-15 Thread Eric Auger
Hi Arnd,
On 10/15/2015 02:12 PM, Christoffer Dall wrote:
> On Thu, Oct 15, 2015 at 01:21:55PM +0200, Arnd Bergmann wrote:
>> On Thursday 15 October 2015 10:08:02 Eric Auger wrote:
>>> Hi Arnd,
>>> On 10/14/2015 05:38 PM, Arnd Bergmann wrote:
 On Wednesday 14 October 2015 15:33:12 Eric Auger wrote:
> --- a/drivers/vfio/platform/vfio_platform_common.c
> +++ b/drivers/vfio/platform/vfio_platform_common.c
> @@ -31,6 +31,11 @@ static const struct vfio_platform_reset_combo 
> reset_lookup_table[] = {
> .reset_function_name = "vfio_platform_calxedaxgmac_reset",
> .module_name = "vfio-platform-calxedaxgmac",
> },
> +   {
> +   .compat = "amd,xgbe-seattle-v1a",
> +   .reset_function_name = "vfio_platform_amdxgbe_reset",
> +   .module_name = "vfio-platform-amdxgbe",
> +   },
>  };
>  
>  static void vfio_platform_get_reset(struct vfio_platform_device *vdev,
>

 This is causing build errors for me when CONFIG_MODULES is disabled.
>>> Sorry about that and thanks for reporting the issue

 Could this please be restructured so vfio_platform_get_reset does
 not attempt to call __symbol_get() but instead has the drivers
 register themselves properly to a subsystem?
>>> OK
>>>
>>> Could you elaborate about "has the drivers register themselves properly
>>> to a subsystem".
>>>
>>> My first proposal when coping with this problematic of being able to add
>>> reset plugins to the vfio-platform driver was to create new drivers per
>>> device requiring reset. but this was considered painful for end-users,
>>> who needed to be aware of the right driver to bind - and I think that
>>> makes sense - (https://lkml.org/lkml/2015/4/17/568) .
>>
>> Having multiple drivers indeed sucks, but your current approach isn't
>> that much better, as you still have two modules that are used to driver
>> the same hardware.
>>
>> I would expect that the same driver that is used for the normal
>> operation and that it calls a function to register itself to vfio
>> by passing a structure with the device and reset function pointer.
>>
>>> A naive question I dare to ask, wouldn't it be acceptable to make
>>> vfio_platform depend on CONFIG_MODULES? Don't we disable modules for
>>> security purpose? In that context would we use VFIO?
>>
>> I think a lot of embedded systems turn off modules to save a little
>> memory, speed up boot time and simplify their user space.
>>
>> Aside from that, the current method is highly unusual and looks a bit
>> fragile to me, as you are relying on internals of the module loader
>> code. It's also a layering violation as the generic code needs to be
>> patched for each device specific module that is added, and we try
>> to avoid that.
Many thanks for taking the time to write this down
>>
>> A possible solution could be something inside the xgbe driver like
>>
>>
>> static void xgbe_init_module(void)
>> {
>>  int ret = 0;
>>
>>  if (IS_ENABLED(CONFIG_AMD_XGBE_ETHERNET)
>>  ret = platform_driver_register(_driver);
>>  if (ret)
>>  return ret;
>>
>>  if (IS_ENABLED(CONFIG_VFIO_PLATFORM))
>>  ret = vfio_platform_register_reset(_of_match, 
>> xgbe_platform_reset);
>>
>>  return ret; 
>> }
>>
>> This way you have exactly one driver module that gets loaded for the
>> device and you can use it either with the platform_driver or through
>> vfio.
If I understand it correctly you still need 2 loaded modules (VFIO
driver & XGBE driver which implements the reset function) or am I
missing something?

I had a similar mechanism of registration in my PATCH v1 but I did the
registration from the reset module itself instead of in the native
driver, as you suggest here.

Best Regards

Eric

>>
>> A nicer way that would be a little more work would be to integrate
>> the reset infrastructure into 'struct platform_driver' framework,
>> by adding another callback to the it for doing the interaction with
>> vfio, something like
>>
>> enum vfio_platform_op {
>>  VFIO_PLATFORM_BIND,
>>  VFIO_PLATFORM_UNBIND,
>>  VFIO_PLATFORM_RESET,
>> };
>>
>> struct platform_driver {
>> int (*probe)(struct platform_device *);
>> int (*remove)(struct platform_device *);
>>  ...
>>  int (*vfio_manage)(struct platform_device *, enum vfio_platform_op);
>> struct device_driver driver;
>> };
>>
>> This would integrate much more closely into the platform driver framework,
>> just like the regular vfio driver integrates into the PCI framework.
>> Unlike PCI however, you can't just use the generic driver framework to
>> unbind the driver, because you still need device specific code.
>>
> Thanks for these suggestions, really helpful.
> 
> What I don't understand in the latter example is how VFIO knows which
> struct platform_driver to interact with?
> 
> Also, just so I'm sure I understand 

[PATCH 2/9] KVM: arm: Do not indent the arguments of DECLARE_BITMAP

2015-10-15 Thread Michal Marek
Besides being a coding style issue, it confuses make tags:

ctags: Warning: include/kvm/arm_vgic.h:307: null expansion of name pattern "\1"
ctags: Warning: include/kvm/arm_vgic.h:308: null expansion of name pattern "\1"
ctags: Warning: include/kvm/arm_vgic.h:309: null expansion of name pattern "\1"
ctags: Warning: include/kvm/arm_vgic.h:317: null expansion of name pattern "\1"

Cc: kvmarm@lists.cs.columbia.edu
Signed-off-by: Michal Marek 
---
 include/kvm/arm_vgic.h | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index d901f1a47be6..58c5c0eb8d2d 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -304,9 +304,9 @@ struct vgic_cpu {
u8  *vgic_irq_lr_map;
 
/* Pending/active/both interrupts on this VCPU */
-   DECLARE_BITMAP( pending_percpu, VGIC_NR_PRIVATE_IRQS);
-   DECLARE_BITMAP( active_percpu, VGIC_NR_PRIVATE_IRQS);
-   DECLARE_BITMAP( pend_act_percpu, VGIC_NR_PRIVATE_IRQS);
+   DECLARE_BITMAP(pending_percpu, VGIC_NR_PRIVATE_IRQS);
+   DECLARE_BITMAP(active_percpu, VGIC_NR_PRIVATE_IRQS);
+   DECLARE_BITMAP(pend_act_percpu, VGIC_NR_PRIVATE_IRQS);
 
/* Pending/active/both shared interrupts, dynamically sized */
unsigned long   *pending_shared;
@@ -314,7 +314,7 @@ struct vgic_cpu {
unsigned long   *pend_act_shared;
 
/* Bitmap of used/free list registers */
-   DECLARE_BITMAP( lr_used, VGIC_V2_MAX_LRS);
+   DECLARE_BITMAP(lr_used, VGIC_V2_MAX_LRS);
 
/* Number of list registers on this CPU */
int nr_lr;
-- 
2.1.4

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH] VFIO: platform: AMD xgbe reset module

2015-10-15 Thread Alex Williamson
On Thu, 2015-10-15 at 21:42 +0200, Christoffer Dall wrote:
> On Thu, Oct 15, 2015 at 10:53:17AM -0600, Alex Williamson wrote:
> > On Thu, 2015-10-15 at 16:46 +0200, Eric Auger wrote:
> > > Hi Arnd,
> > > On 10/15/2015 03:59 PM, Arnd Bergmann wrote:
> > > > On Thursday 15 October 2015 14:12:28 Christoffer Dall wrote:
> > > >>>
> > > >>> enum vfio_platform_op {
> > > >>>   VFIO_PLATFORM_BIND,
> > > >>>   VFIO_PLATFORM_UNBIND,
> > > >>>   VFIO_PLATFORM_RESET,
> > > >>> };
> > > >>>
> > > >>> struct platform_driver {
> > > >>> int (*probe)(struct platform_device *);
> > > >>> int (*remove)(struct platform_device *);
> > > >>>   ...
> > > >>>   int (*vfio_manage)(struct platform_device *, enum 
> > > >>> vfio_platform_op);
> > > >>> struct device_driver driver;
> > > >>> };
> > > >>>
> > > >>> This would integrate much more closely into the platform driver 
> > > >>> framework,
> > > >>> just like the regular vfio driver integrates into the PCI framework.
> > > >>> Unlike PCI however, you can't just use the generic driver framework to
> > > >>> unbind the driver, because you still need device specific code.
> > > >>>
> > > >> Thanks for these suggestions, really helpful.
> > > >>
> > > >> What I don't understand in the latter example is how VFIO knows which
> > > >> struct platform_driver to interact with?
> > > > 
> > > > This would assume that the driver remains bound to the device, so VFIO
> > > > gets a pointer to the device from somewhere (as it does today) and then
> > > > follows the dev->driver pointer to get to the platform_driver.
> > 
> > The complexity of managing a bi-modal driver seems like far more than a
> > little bit of code duplication in a device specific reset module and
> > extends into how userspace makes devices available through vfio, so I
> > think it's too late for that discussion.
> >   
> 
> I have had extremely limited exposure to the implementation details of
> the drivers for devices relevant for VFIO platform, so apologies for
> asking stupid questions.
> 
> I'm sure that your point is valid, I just fully understand how the
> complexities of a bi-modal driver arise?
> 
> Is it simply that the reset function in a particular device driver may
> not be self-contained so therefore the whole driver would need to be
> refactored to be able to do a reset for the purpose of VFIO?

Yes, I would expect that reset function in a driver is not typically
self contained, probably referencing driver specific data structures for
register offsets, relying on various mappings that are expected to be
created from the driver probe() function, etc.  It also creates a
strange dependency on the host driver, how is the user to know they need
the native host driver loaded for full functionality in a device
assignment scenario?  Are we going to need to do a module_request() of
the host driver in vfio platform anyway?  What if there are multiple
devices and the host driver claims the others when loaded?  In the case
of PCI and SR-IOV virtual functions, I often blacklist the host VF
driver because I shouldn't need it when I only intend to use the device
in guests.  Not to mention that we'd have to drop a little bit of vfio
knowledge into each host driver that we intend to enlighten like this,
and how do we resolve whether the host driver, potentially compiled from
a separate source tree has this support.

I really don't see the layering violation in having a set of reset
functions and some lookup mechanism to pick the correct one.  The vfio
platform driver is a meta driver and sometimes we need to enlighten it a
little bit more about the device it's operating on.  For PCI we have all
sorts of special functionality for reset, but we have a standard to work
with there, so we may need to choose between a bus reset, PM reset, AF
FLR, PCIe FLR, or device specific reset, but it's all buried in the PCI
core code; where device specific resets are the exception on PCI, they
are the norm on platform.

> > > >> Also, just so I'm sure I understand correctly, VFIO_PLATFORM_UNBIND is
> > > >> then called by VFIO before the VFIO driver unbinds from the device
> > > >> (unbinding the platform driver from the device being a completely
> > > >> separate thing)?
> > > > 
> > > > This is where we'd need a little more changes for this approach. Instead
> > > > of unbinding the device from its driver, the idea would be that the
> > > > driver remains bound as far as the driver model is concerned, but
> > > > it would be in a quiescent state where no other subsystem interacts with
> > > > it (i.e. it gets unregistered from networking core or whichever it 
> > > > uses).
> > > 
> > > Currently we use the same mechanism as for PCI, ie. unbind the native
> > > driver and then bind VFIO platform driver in its place. Don't you think
> > > changing this may be a pain for user-space tools that are designed to
> > > work that way for PCI?
> > > 
> > > My personal preference would be to start 

Re: [PATCH v3 00/20] KVM: ARM64: Add guest PMU support

2015-10-15 Thread Wei Huang


On 09/24/2015 05:31 PM, Shannon Zhao wrote:
> This patchset adds guest PMU support for KVM on ARM64. It takes
> trap-and-emulate approach. When guest wants to monitor one event, it
> will be trapped by KVM and KVM will call perf_event API to create a perf
> event and call relevant perf_event APIs to get the count value of event.
> 
> Use perf to test this patchset in guest. When using "perf list", it
> shows the list of the hardware events and hardware cache events perf
> supports. Then use "perf stat -e EVENT" to monitor some event. For
> example, use "perf stat -e cycles" to count cpu cycles and
> "perf stat -e cache-misses" to count cache misses.
> 
> Below are the outputs of "perf stat -r 5 sleep 5" when running in host
> and guest.
> 
> Host:
>  Performance counter stats for 'sleep 5' (5 runs):
> 
>   0.551428  task-clock (msec) #0.000 CPUs utilized
> ( +-  0.91% )
>  1  context-switches  #0.002 M/sec
>  0  cpu-migrations#0.000 K/sec
> 48  page-faults   #0.088 M/sec
> ( +-  1.05% )
>1150265  cycles#2.086 GHz  
> ( +-  0.92% )
>  stalled-cycles-frontend
>  stalled-cycles-backend
> 526398  instructions  #0.46  insns per cycle  
> ( +-  0.89% )
>  branches
>   9485  branch-misses #   17.201 M/sec
> ( +-  2.35% )
> 
>5.000831616 seconds time elapsed   
>( +-  0.00% )
> 
> Guest:
>  Performance counter stats for 'sleep 5' (5 runs):
> 
>   0.730868  task-clock (msec) #0.000 CPUs utilized
> ( +-  1.13% )
>  1  context-switches  #0.001 M/sec
>  0  cpu-migrations#0.000 K/sec
> 48  page-faults   #0.065 M/sec
> ( +-  0.42% )
>1642982  cycles#2.248 GHz  
> ( +-  1.04% )
>  stalled-cycles-frontend
>  stalled-cycles-backend
> 637964  instructions  #0.39  insns per cycle  
> ( +-  0.65% )
>  branches
>  10377  branch-misses #   14.198 M/sec
> ( +-  1.09% )
> 
>5.001289068 seconds time elapsed   
>( +-  0.00% )
> 

Thanks for V3. One suggestion is to run more perf stress tests, such as
"perf test". So we know the corner cases are covered as much as possible.

> This patchset can be fetched from [1] and the relevant QEMU version for
> test can be fetched from [2].
> 
> Thanks,
> Shannon
> 
> [1] https://git.linaro.org/people/shannon.zhao/linux-mainline.git  
> KVM_ARM64_PMU_v3
> [2] https://git.linaro.org/people/shannon.zhao/qemu.git  PMU_v2
> 
> Changes since v2->v3:
> * Directly use perf raw event type to create perf_event in KVM
> * Add a helper vcpu_sysreg_write
> * remove unrelated header file
> 
> Changes since v1->v2:
> * Use switch...case for registers access handler instead of adding
>   alone handler for each register
> * Try to use the sys_regs to store the register value instead of adding
>   new variables in struct kvm_pmc
> * Fix the handle of cp15 regs
> * Create a new kvm device vPMU, then userspace could choose whether to
>   create PMU
> * Fix the handle of PMU overflow interrupt
> 
> Shannon Zhao (20):
>   ARM64: Move PMU register related defines to asm/pmu.h
>   KVM: ARM64: Define PMU data structure for each vcpu
>   KVM: ARM64: Add offset defines for PMU registers
>   KVM: ARM64: Add reset and access handlers for PMCR_EL0 register
>   KVM: ARM64: Add reset and access handlers for PMSELR register
>   KVM: ARM64: Add reset and access handlers for PMCEID0 and PMCEID1
> register
>   KVM: ARM64: PMU: Add perf event map and introduce perf event creating
> function
>   KVM: ARM64: Add reset and access handlers for PMXEVTYPER register
>   KVM: ARM64: Add reset and access handlers for PMXEVCNTR register
>   KVM: ARM64: Add reset and access handlers for PMCCNTR register
>   KVM: ARM64: Add reset and access handlers for PMCNTENSET and
> PMCNTENCLR register
>   KVM: ARM64: Add reset and access handlers for PMINTENSET and
> PMINTENCLR register
>   KVM: ARM64: Add reset and access handlers for PMOVSSET and PMOVSCLR
> register
>   KVM: ARM64: Add reset and access handlers for PMUSERENR register
>   KVM: ARM64: Add reset and access handlers for PMSWINC register
>   KVM: ARM64: Add access handlers for PMEVCNTRn and PMEVTYPERn register
>   KVM: ARM64: Add PMU overflow interrupt routing
>   KVM: ARM64: Reset PMU state when resetting vcpu
>   KVM: ARM64: Free perf event of PMU when destroying vcpu
>   KVM: ARM64: Add a new kvm ARM PMU device
> 
>  

Re: [PATCH] VFIO: platform: AMD xgbe reset module

2015-10-15 Thread Christoffer Dall
On Thu, Oct 15, 2015 at 5:49 PM, Arnd Bergmann  wrote:
> On Thursday 15 October 2015 17:03:21 Christoffer Dall wrote:
>> On Thu, Oct 15, 2015 at 04:55:13PM +0200, Arnd Bergmann wrote:
>> > On Thursday 15 October 2015 16:46:09 Eric Auger wrote:
>> > > >
>> > > > This is where we'd need a little more changes for this approach. 
>> > > > Instead
>> > > > of unbinding the device from its driver, the idea would be that the
>> > > > driver remains bound as far as the driver model is concerned, but
>> > > > it would be in a quiescent state where no other subsystem interacts 
>> > > > with
>> > > > it (i.e. it gets unregistered from networking core or whichever it 
>> > > > uses).
>> > >
>> > > Currently we use the same mechanism as for PCI, ie. unbind the native
>> > > driver and then bind VFIO platform driver in its place. Don't you think
>> > > changing this may be a pain for user-space tools that are designed to
>> > > work that way for PCI?
>> > >
>> > > My personal preference would be to start with your first proposal since
>> > > it looks (to me) less complex and "unknown" that the 2d approach.
>> >
>> > We certainly can't easily change from one approach to the other without
>> > breaking user expectations, so the decision needs to be made carefully.
>> >
>> > The main observation here is that platform devices are unlike PCI in this
>> > regard because they need extra per-device code. I have argued in the
>> > past that we should not reuse the "VFIO" name here because it's actually
>> > something else.
>>
>> I've adjusted to consider VFIO a general purpose framework for mapping
>> device resources into userspace/VMs, and there are certainly a lot of
>> commonality with both PCI, platform, and potentially other devices for
>> that to make sense.
>>
>>
>> > On the other hand, there are a lot of commonalities,
>> > we just have to make sure we don't try to force the code into one model
>> > that doesn't really work just to make it look more like PCI VFIO.
>> >
>>
>> But given that we now have code for platform device passthrough that
>> works in both QEMU and the kernel side and is actually useful for
>> people, is there a clear technical advantage to go back and rework thaat
>> at this point?
>>
>> Don't get me wrong, I like the idea of having a single driver bound to a
>> platform device, and then that's it, but it just feels like that
>> discussion doesn't necessarily belong in the context of a patch that
>> 'just' seeks to add reset functionality for a specific device for VFIO?
>
> Ah, this is for qemu/kvm? If there is already upstream qemu code that
> finds it easier to use this approach, it's certainly more logical to
> deal with it in my first approach than the second.

yes, indeed.

>
> I was thinking of ODP as the primary user, and that wouldn't need
> the interface to be consistent with PCI as much, because the code
> is inherently device specific there anyway.
>

I didn't think about how this applies to ODP (haven't had time to look
at ODP in any technical detail TBH), but it sounds like ODP could be
adapted to use either approach.

In any case we can always transition towards your 2nd approach in a
userspace compatible way later, if we find good reasons to.

Thanks,
-Christoffer
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH] VFIO: platform: AMD xgbe reset module

2015-10-15 Thread Alex Williamson
On Thu, 2015-10-15 at 16:46 +0200, Eric Auger wrote:
> Hi Arnd,
> On 10/15/2015 03:59 PM, Arnd Bergmann wrote:
> > On Thursday 15 October 2015 14:12:28 Christoffer Dall wrote:
> >>>
> >>> enum vfio_platform_op {
> >>>   VFIO_PLATFORM_BIND,
> >>>   VFIO_PLATFORM_UNBIND,
> >>>   VFIO_PLATFORM_RESET,
> >>> };
> >>>
> >>> struct platform_driver {
> >>> int (*probe)(struct platform_device *);
> >>> int (*remove)(struct platform_device *);
> >>>   ...
> >>>   int (*vfio_manage)(struct platform_device *, enum vfio_platform_op);
> >>> struct device_driver driver;
> >>> };
> >>>
> >>> This would integrate much more closely into the platform driver framework,
> >>> just like the regular vfio driver integrates into the PCI framework.
> >>> Unlike PCI however, you can't just use the generic driver framework to
> >>> unbind the driver, because you still need device specific code.
> >>>
> >> Thanks for these suggestions, really helpful.
> >>
> >> What I don't understand in the latter example is how VFIO knows which
> >> struct platform_driver to interact with?
> > 
> > This would assume that the driver remains bound to the device, so VFIO
> > gets a pointer to the device from somewhere (as it does today) and then
> > follows the dev->driver pointer to get to the platform_driver.

The complexity of managing a bi-modal driver seems like far more than a
little bit of code duplication in a device specific reset module and
extends into how userspace makes devices available through vfio, so I
think it's too late for that discussion.
  
> >> Also, just so I'm sure I understand correctly, VFIO_PLATFORM_UNBIND is
> >> then called by VFIO before the VFIO driver unbinds from the device
> >> (unbinding the platform driver from the device being a completely
> >> separate thing)?
> > 
> > This is where we'd need a little more changes for this approach. Instead
> > of unbinding the device from its driver, the idea would be that the
> > driver remains bound as far as the driver model is concerned, but
> > it would be in a quiescent state where no other subsystem interacts with
> > it (i.e. it gets unregistered from networking core or whichever it uses).
> 
> Currently we use the same mechanism as for PCI, ie. unbind the native
> driver and then bind VFIO platform driver in its place. Don't you think
> changing this may be a pain for user-space tools that are designed to
> work that way for PCI?
> 
> My personal preference would be to start with your first proposal since
> it looks (to me) less complex and "unknown" that the 2d approach.
> 
> Let's wait for Alex opinion too...

I thought the reason we took the approach we have now is so that we
don't have reset code loaded into the kernel unless we have a device
that needs it.  Therefore we don't really want to preemptively load all
the reset drivers and have them do a registration.  The unfortunate
side-effect of that is the platform code needs to go looking for the
driver.  We do that via the __symbol_get() trick, which only fails
without modules because the underscore variant isn't defined in that
case.  I remember asking Eric previously why we're using that rather
than symbol_get(), I've since forgotten his answer, but the fact that
__symbol_get() is only defined for modules makes it moot, we either need
to make symbol_get() work or define __symbol_get() for non-module
builds.

Otherwise, we should probably abandon the idea of these reset functions
being modules and build them into the vfio platform driver (which would
still be less loaded, dead code than a bi-modal host driver).  Thanks,

Alex

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: its_alloc_tables() - with all BASER marked none

2015-10-15 Thread Mario Smarduch


On 10/15/2015 12:50 AM, Marc Zyngier wrote:
> On 15/10/15 02:30, Mario Smarduch wrote:
>> If its_init()/its_probe()/its_alloc_tables() finds all GITS_BASER type none, 
>> it
>> continues with ITS initialization.
>>
>> Is there a reason to continue?
> 
> Of course. There is nothing that *mandates* the ITS to request memory
> from the operating system. The HW could perfectly come with its own
> memory and not need anything at all.
> 
>> Started to look through this code recently - little confused here.
> 
> Only the beginning! ;-)

Well at this time the beginning of console freeze close to  "ITS: using cache
flushing for cmd queue". Poked around CBASER and noticed Inner cache bits are
updated but not sticky so it got me wondering.

Appears like I need to look elsewhere, like maybe my environment.

Thanks for clarifying.

- Mario

> 
>   M.
> 
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: its_alloc_tables() - with all BASER marked none

2015-10-15 Thread Mario Smarduch


On 10/15/2015 10:10 AM, Marc Zyngier wrote:
> On 15/10/15 18:08, Mario Smarduch wrote:
>>
>>
>> On 10/15/2015 12:50 AM, Marc Zyngier wrote:
>>> On 15/10/15 02:30, Mario Smarduch wrote:
 If its_init()/its_probe()/its_alloc_tables() finds all GITS_BASER type 
 none, it
 continues with ITS initialization.

 Is there a reason to continue?
>>>
>>> Of course. There is nothing that *mandates* the ITS to request memory
>>> from the operating system. The HW could perfectly come with its own
>>> memory and not need anything at all.
>>>
 Started to look through this code recently - little confused here.
>>>
>>> Only the beginning! ;-)
>>
>> Well at this time the beginning of console freeze close to  "ITS: using cache
>> flushing for cmd queue". Poked around CBASER and noticed Inner cache bits are
>> updated but not sticky so it got me wondering.
>>
>> Appears like I need to look elsewhere, like maybe my environment.
> 
> Out of curiosity, is that on a FastModel? Or on some actual HW?

FastModel, license ran out on FVP.
> 
>   M.
> 
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH] VFIO: platform: AMD xgbe reset module

2015-10-15 Thread Christoffer Dall
On Thu, Oct 15, 2015 at 10:53:17AM -0600, Alex Williamson wrote:
> On Thu, 2015-10-15 at 16:46 +0200, Eric Auger wrote:
> > Hi Arnd,
> > On 10/15/2015 03:59 PM, Arnd Bergmann wrote:
> > > On Thursday 15 October 2015 14:12:28 Christoffer Dall wrote:
> > >>>
> > >>> enum vfio_platform_op {
> > >>>   VFIO_PLATFORM_BIND,
> > >>>   VFIO_PLATFORM_UNBIND,
> > >>>   VFIO_PLATFORM_RESET,
> > >>> };
> > >>>
> > >>> struct platform_driver {
> > >>> int (*probe)(struct platform_device *);
> > >>> int (*remove)(struct platform_device *);
> > >>>   ...
> > >>>   int (*vfio_manage)(struct platform_device *, enum 
> > >>> vfio_platform_op);
> > >>> struct device_driver driver;
> > >>> };
> > >>>
> > >>> This would integrate much more closely into the platform driver 
> > >>> framework,
> > >>> just like the regular vfio driver integrates into the PCI framework.
> > >>> Unlike PCI however, you can't just use the generic driver framework to
> > >>> unbind the driver, because you still need device specific code.
> > >>>
> > >> Thanks for these suggestions, really helpful.
> > >>
> > >> What I don't understand in the latter example is how VFIO knows which
> > >> struct platform_driver to interact with?
> > > 
> > > This would assume that the driver remains bound to the device, so VFIO
> > > gets a pointer to the device from somewhere (as it does today) and then
> > > follows the dev->driver pointer to get to the platform_driver.
> 
> The complexity of managing a bi-modal driver seems like far more than a
> little bit of code duplication in a device specific reset module and
> extends into how userspace makes devices available through vfio, so I
> think it's too late for that discussion.
>   

I have had extremely limited exposure to the implementation details of
the drivers for devices relevant for VFIO platform, so apologies for
asking stupid questions.

I'm sure that your point is valid, I just fully understand how the
complexities of a bi-modal driver arise?

Is it simply that the reset function in a particular device driver may
not be self-contained so therefore the whole driver would need to be
refactored to be able to do a reset for the purpose of VFIO?

> > >> Also, just so I'm sure I understand correctly, VFIO_PLATFORM_UNBIND is
> > >> then called by VFIO before the VFIO driver unbinds from the device
> > >> (unbinding the platform driver from the device being a completely
> > >> separate thing)?
> > > 
> > > This is where we'd need a little more changes for this approach. Instead
> > > of unbinding the device from its driver, the idea would be that the
> > > driver remains bound as far as the driver model is concerned, but
> > > it would be in a quiescent state where no other subsystem interacts with
> > > it (i.e. it gets unregistered from networking core or whichever it uses).
> > 
> > Currently we use the same mechanism as for PCI, ie. unbind the native
> > driver and then bind VFIO platform driver in its place. Don't you think
> > changing this may be a pain for user-space tools that are designed to
> > work that way for PCI?
> > 
> > My personal preference would be to start with your first proposal since
> > it looks (to me) less complex and "unknown" that the 2d approach.
> > 
> > Let's wait for Alex opinion too...
> 
> I thought the reason we took the approach we have now is so that we
> don't have reset code loaded into the kernel unless we have a device
> that needs it.  Therefore we don't really want to preemptively load all
> the reset drivers and have them do a registration.  The unfortunate
> side-effect of that is the platform code needs to go looking for the
> driver. 

Does the current approach have a separate driver for doing VFIO reset or
does it reuse the existing device driver?

Why does the driver registering itself instead of using symbol_get
imply that we must load reset drivers that we don't need?

Thanks,
-Christoffer


> We do that via the __symbol_get() trick, which only fails
> without modules because the underscore variant isn't defined in that
> case.  I remember asking Eric previously why we're using that rather
> than symbol_get(), I've since forgotten his answer, but the fact that
> __symbol_get() is only defined for modules makes it moot, we either need
> to make symbol_get() work or define __symbol_get() for non-module
> builds.
> 
> Otherwise, we should probably abandon the idea of these reset functions
> being modules and build them into the vfio platform driver (which would
> still be less loaded, dead code than a bi-modal host driver).  Thanks,
> 
> Alex
> 
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm