[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 06/11] x86/hvm: allowing registering EOI callbacks for GSIs


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Thu, 8 Apr 2021 17:06:41 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=7CgU6nSaT69CiB5OK9+hxYwcWyQhI/cL9kFgpaJrnj8=; b=mgoUIdfICHkkQC8UwIiHKKrvjutM8gNNVGp+lKcmuV5VLOtZSDdgNk/yv7muXB98GQ9SQmEGBpgXGyqllXPkpEpmBRjRtJZQKX7js4038ta0dGRnQdRJtk1PuoPmOOxXWZYOBM7sqg23KJ3kZlceH6YZ+dK3iF9Y8n4lk8bJBy3q6T1mocwDcwyYpuyTFDL7IFDGazA9OZddFEcLcm58DYMv1S2Zg0VablMjO2TqRCsCkBQTmBriDSaz9+3n3ug07tTKWaZ6W0BIkI9eLEpNrNkPFTVbnwGweHYIM7tDqWb6Wexh3+YDWqFiQyLb3jItJZNmzPdrzTBMuR+hNYszBw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=HT8Dmultv7mxzDJpuSVY6GsN8eyL+VxX5AVJ5FCgKRvM4d/DjzFhtKaAekzi/nsaomYgDUOgs567r+vARvXPGI41/87fyFAngUIx2We4Jr+xIfOVY0LFbbXjIweWbKbspYp5yAnaupwd8SlHEZ8jNBKNDGvfjcgnq+5KLt2CKmSCscuSbh7C8bIrNNEnxXDfr8EuHM5yEow/2g7WK8om8HtwYLg2tUEWlwRD+uKOlmPAO2sCxpryd9gDodtc0hpOAobyQACpUwVbVb22TCta4FZN4iz0450VbPdqUV0t7aH3xlQW5ay4srPTHZjZ9MdLSOhSBGOLN8ZLsixAAN+KQg==
  • Authentication-results: esa3.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Thu, 08 Apr 2021 15:07:09 +0000
  • Ironport-hdrordr: A9a23:BgGWvqtFHGQoUHUCe4AbtHxm7skCpIMji2hD6mlwRA09T+Wxi9 2ukPMH1RX9lTYWXzUalcqdPbSbKEmwybde6+AqXYuKdg/gpWeuMcVe/ZLvqgeQfxHW28x88e Nbc6Z4AMDtFlQSt6zHySSxDtpI+ra62Y+yg+O29RlQZCVLT40l0AtjEAacFSRNNWx7LL40Do CV6MYChxfIQwV1Uu2BCnMIX/fOqrTw/fqLXTc8CwM68w7LtDu06dfBeCSw5AsUUD9E3N4ZgA r4ujH+j5/Dj9iLjiXy+kWWxJRQldvnxLJ4dbGxo/lQEBrAoEKFYIxgXpeLoTw6pvq+gWxa7O XkklMbEOlYr1/XY2GpsTvhsjOQrwoG2jvZ5nKzxVvlvMDjSzoxB6N69PxkWyqc0WUMlpVY8s twrgWknqsSNzzstmDBwuKNcBdQmk+9sRMZ4JUupk0aa6QyQvtst4AFlXkldqsoLWbBx60MNv JhN83Y7OY+SyLhU1np+lNix9GhRR0Ib267a3lHvsSU1g5fl2xiw30Zw9QCnmwB+IhVceg929 j5
  • Ironport-sdr: /je1AXmk2CpMdN3X02QpJsZtPjjr7Nx2bt/SoUs0cOEqgp5B+kRDqB1Fi4PyNJ3BKn4xcMruY2 XPPvvh4EXJWLlk8x9oTP1NDbJOycmXzVwmrLp4SOz9Eosp+MimuP0pvV/ONYcKqthJr2l9vfsw Gy5bSFLdyjYmkLaMBVJh+OAKK4wpYQH0MWUzdQ+bZQ5dkdIV6Vir6baLKMrj1FTf9e/J4pY+hc DciDxAG6FOh4B3jQtFRjgyED0ajfVhZHgsd6GlzX0ao7zi3hxGKkPFZrrDFZZj+nflzraIR+Kq rRw=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Thu, Apr 08, 2021 at 04:31:59PM +0200, Jan Beulich wrote:
> On 08.04.2021 14:52, Roger Pau Monné wrote:
> > On Wed, Apr 07, 2021 at 05:51:14PM +0200, Jan Beulich wrote:
> >> On 31.03.2021 12:32, Roger Pau Monne wrote:
> >>> --- a/xen/arch/x86/hvm/irq.c
> >>> +++ b/xen/arch/x86/hvm/irq.c
> >>> +void hvm_gsi_unregister_callback(struct domain *d, unsigned int gsi,
> >>> +                                 struct hvm_gsi_eoi_callback *cb)
> >>> +{
> >>> +    struct hvm_irq *hvm_irq = hvm_domain_irq(d);
> >>> +    const struct list_head *tmp;
> >>> +
> >>> +    if ( gsi >= hvm_irq->nr_gsis )
> >>> +    {
> >>> +        ASSERT_UNREACHABLE();
> >>> +        return;
> >>> +    }
> >>> +
> >>> +    write_lock(&hvm_irq->gsi_callbacks_lock);
> >>> +    list_for_each ( tmp, &hvm_irq->gsi_callbacks[gsi] )
> >>> +        if ( tmp == &cb->list )
> >>> +        {
> >>> +            list_del(&cb->list);
> >>> +            break;
> >>> +        }
> >>> +    write_unlock(&hvm_irq->gsi_callbacks_lock);
> >>> +}
> >>
> >> Perhaps somehow flag, at least in debug builds, if the callback
> >> wasn#t found?
> > 
> > I've added a debug printf here to warn if the callback is not found,
> > but I see it triggering because hpet_set_timer will call
> > destroy_periodic_time and create_periodic_time and thus two calls will
> > be made to hvm_gsi_unregister_callback. This is fine, but adding a
> > message there gets too verbose, so I will drop it and leave the code
> > as-is.
> > 
> > I don't see a problem with calling destroy_periodic_time multiple
> > times even if the timer was not active, and that shouldn't result in a
> > message being printed.
> 
> If destroy_periodic_time() is to remain the only caller, I guess I
> agree. Other (future) callers may then need this function to gain
> a return value indicating whether the callback was actually found.

There's also pt_irq_destroy_bind which likely cares about the return
value, so let's return a value from hvm_gsi_unregister_callback and
check it in pt_irq_destroy_bind.

Thanks, Roger.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.