>>> On 12.09.18 at 11:10, <[email protected]> wrote:
>> From: Xen-devel [mailto:[email protected]] On Behalf
>> Of Jan Beulich
>> Sent: 11 September 2018 15:56
>>
>> >>> On 23.08.18 at 11:47, <[email protected]> wrote:
>> > ...for some uses of get_page_from_gfn().
>> >
>> > There are many occurences of the following pattern in the code:
>> >
>> > q = <readonly look-up> ? P2M_ALLOC : P2M_UNSHARE;
>>
>> Especially with this UNSHARE in mind - is "paged" in the helper
>> function's name really suitable? Since we (I think) already have
>> get_gfn(), how about try_get_gfn()?
>
> That name may be a little misleading since it suggests a close functional
> relationship with get_gfn() whereas it does more than that. How about
> try_get_page_from_gfn()?
Fine with me; George?
>> > --- a/xen/arch/x86/hvm/emulate.c
>> > +++ b/xen/arch/x86/hvm/emulate.c
>> > @@ -350,34 +350,16 @@ static int hvmemul_do_io_buffer(
>> >
>> > static int hvmemul_acquire_page(unsigned long gmfn, struct page_info
>> **page)
>> > {
>> > - struct domain *curr_d = current->domain;
>> > - p2m_type_t p2mt;
>> > -
>> > - *page = get_page_from_gfn(curr_d, gmfn, &p2mt, P2M_UNSHARE);
>> > -
>> > - if ( *page == NULL )
>> > - return X86EMUL_UNHANDLEABLE;
>> > -
>> > - if ( p2m_is_paging(p2mt) )
>> > - {
>> > - put_page(*page);
>> > - p2m_mem_paging_populate(curr_d, gmfn);
>> > - return X86EMUL_RETRY;
>> > - }
>> > -
>> > - if ( p2m_is_shared(p2mt) )
>> > + switch ( get_paged_gfn(current->domain, _gfn(gmfn), false, NULL,
>> page) )
>> > {
>> > - put_page(*page);
>> > + case -EAGAIN:
>> > return X86EMUL_RETRY;
>> > - }
>> > -
>> > - /* This code should not be reached if the gmfn is not RAM */
>> > - if ( p2m_is_mmio(p2mt) )
>> > - {
>> > - domain_crash(curr_d);
>> > -
>> > - put_page(*page);
>> > + case -EINVAL:
>> > return X86EMUL_UNHANDLEABLE;
>> > + default:
>> > + ASSERT_UNREACHABLE();
>> > + case 0:
>>
>> I think you'd better have "default:" fall through to "case -EINVAL".
>> Similarly elsewhere.
>
> Ok. I'll keep the ASSERT_UNREACHABLE() though.
That's what I was implying by saying "fall through" - otherwise
"case -EINVAL:" could as well have gone away.
Jan
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
