From: Matias Ezequiel Vara Larsen <matiasevara@gmail.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: "Matias Ezequiel Vara Larsen" <matias.vara@vates.fr>,
"Andrew Cooper" <andrew.cooper3@citrix.com>,
"Roger Pau Monné" <roger.pau@citrix.com>, "Wei Liu" <wl@xen.org>,
"George Dunlap" <george.dunlap@citrix.com>,
"Julien Grall" <julien@xen.org>,
"Stefano Stabellini" <sstabellini@kernel.org>,
"Dario Faggioli" <dfaggioli@suse.com>,
xen-devel@lists.xenproject.org
Subject: Re: [RFC PATCH v2 1/2] xen/memory : Add a stats_table resource type
Date: Tue, 7 Mar 2023 15:44:27 +0100 [thread overview]
Message-ID: <20230307144427.GA997565@horizon> (raw)
In-Reply-To: <496b1fd7-4540-66c6-be89-51f20a6666ab@suse.com>
On Thu, Feb 23, 2023 at 01:42:08PM +0100, Jan Beulich wrote:
> On 23.02.2023 13:16, Matias Ezequiel Vara Larsen wrote:
> > On Fri, Feb 17, 2023 at 03:10:53PM +0100, Jan Beulich wrote:
> >> On 17.02.2023 10:29, Matias Ezequiel Vara Larsen wrote:
> >>> On Fri, Feb 17, 2023 at 09:57:43AM +0100, Jan Beulich wrote:
> >>>> On 17.02.2023 09:50, Matias Ezequiel Vara Larsen wrote:
> >>>>> On Wed, Dec 14, 2022 at 08:56:57AM +0100, Jan Beulich wrote:
> >>>>>> On 14.12.2022 08:29, Jan Beulich wrote:
> >>>>>>> On 07.10.2022 14:39, Matias Ezequiel Vara Larsen wrote:
> >>>>>>>> +static int stats_vcpu_alloc_mfn(struct domain *d)
> >>>>>>>> +{
> >>>>>>>> + struct page_info *pg;
> >>>>>>>> +
> >>>>>>>> + pg = alloc_domheap_page(d, MEMF_no_refcount);
> >>>>>>>
> >>>>>>> The ioreq and vmtrace resources are also allocated this way, but they're
> >>>>>>> HVM-specific. The one here being supposed to be VM-type independent, I'm
> >>>>>>> afraid such pages will be accessible by an "owning" PV domain (it'll
> >>>>>>> need to guess the MFN, but that's no excuse).
> >>>>>>
> >>>>>> Which might be tolerable if it then can't write to the page. That would
> >>>>>> require "locking" the page r/o (from guest pov), which ought to be
> >>>>>> possible by leveraging a variant of what share_xen_page_with_guest()
> >>>>>> does: It marks pages PGT_none with a single type ref. This would mean
> >>>>>> ...
> >>>>>>
> >>>>>>>> + if ( !pg )
> >>>>>>>> + return -ENOMEM;
> >>>>>>>> +
> >>>>>>>> + if ( !get_page_and_type(pg, d, PGT_writable_page) ) {
> >>>>>>
> >>>>>> ... using PGT_none here. Afaict this _should_ work, but we have no
> >>>>>> precedent of doing so in the tree, and I may be overlooking something
> >>>>>> which prevents that from working.
> >>>>>>
> >>>>>
> >>>>> I do not fully understand this. I checked share_xen_page_with_guest() and I
> >>>>> think you're talking about doing something like this for each allocated page to
> >>>>> set them ro from a pv guest pov:
> >>>>>
> >>>>> pg->u.inuse.type_info = PGT_none;
> >>>>> pg->u.inuse.type_info |= PGT_validated | 1;
> >>>>> page_set_owner(page, d); // not sure if this is needed
> >>>>>
> >>>>> Then, I should use PGT_none instead of PGT_writable_page in
> >>>>> get_page_and_type(). Am I right?
> >>>>
> >>>> No, if at all possible you should avoid open-coding anything. As said,
> >>>> simply passing PGT_none to get_page_and_type() ought to work (again, as
> >>>> said, unless I'm overlooking something). share_xen_page_with_guest()
> >>>> can do what it does because the page isn't owned yet. For a page with
> >>>> owner you may not fiddle with type_info in such an open-coded manner.
> >>>>
> >>>
> >>> Thanks. I got the following bug when passing PGT_none:
> >>>
> >>> (XEN) Bad type in validate_page 0 t=0000000000000001 c=8040000000000002
> >>> (XEN) Xen BUG at mm.c:2643
> >>
> >> The caller of the function needs to avoid the call not only for writable
> >> and shared pages, but also for this new case of PGT_none.
> >
> > Thanks. If I understand correctly, _get_page_type() needs to avoid to call
> > validate_page() when type = PGT_none.
>
> Yes.
>
> > For the writable and shared pages, this
> > is avoided by setting nx |= PGT_validated. Am I right?
>
> Well, no, I wouldn't describe it like that. The two (soon three) types not
> requiring validation simply set the flag without calling validate_page().
>
I see, thanks. I added the corresponding check at _get_page_type() to set the
flag without calling validate_page() for the PGT_none type. I think I am
missing something when I am releasing the pages. I am triggering the following
BUG() when issuing put_page_and_type():
(XEN) Xen BUG at mm.c:2698
This is at devalidate_page(). I guess the call to devalidate_page() shall be
also avoided. I was wondering if put_page_and_type() is required in this case.
Matias
next prev parent reply other threads:[~2023-03-07 14:44 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-07 12:39 [RFC PATCH v2 0/2] Add a new acquire resource to query vcpu statistics Matias Ezequiel Vara Larsen
2022-10-07 12:39 ` [RFC PATCH v2 1/2] xen/memory : Add a stats_table resource type Matias Ezequiel Vara Larsen
2022-12-13 17:02 ` Jan Beulich
2023-02-16 14:48 ` Matias Ezequiel Vara Larsen
2023-02-16 15:10 ` Jan Beulich
2022-12-14 7:29 ` Jan Beulich
2022-12-14 7:56 ` Jan Beulich
2023-02-17 8:50 ` Matias Ezequiel Vara Larsen
2023-02-17 8:57 ` Jan Beulich
2023-02-17 9:29 ` Matias Ezequiel Vara Larsen
2023-02-17 14:10 ` Jan Beulich
2023-02-23 12:16 ` Matias Ezequiel Vara Larsen
2023-02-23 12:42 ` Jan Beulich
2023-03-07 14:44 ` Matias Ezequiel Vara Larsen [this message]
2023-03-07 16:55 ` Jan Beulich
2023-03-09 9:22 ` Matias Ezequiel Vara Larsen
2023-02-16 15:07 ` Matias Ezequiel Vara Larsen
2023-02-16 15:15 ` Jan Beulich
2023-02-20 16:51 ` Matias Ezequiel Vara Larsen
2023-02-21 8:48 ` Jan Beulich
2022-10-07 12:39 ` [RFC PATCH v2 2/2] tools/misc: Add xen-vcpus-stats tool Matias Ezequiel Vara Larsen
2023-02-23 16:01 ` Andrew Cooper
2023-02-23 20:31 ` Julien Grall
2023-03-17 11:01 ` Matias Ezequiel Vara Larsen
2023-03-29 21:29 ` Julien Grall
2023-02-24 15:31 ` Matias Ezequiel Vara Larsen
2023-02-23 19:56 ` API/ABIs: Re: [RFC PATCH v2 0/2] Add a new acquire resource to query vcpu statistics Andrew Cooper
2023-03-06 14:23 ` Matias Ezequiel Vara Larsen
2023-03-07 10:12 ` Jan Beulich
2023-03-08 11:54 ` Matias Ezequiel Vara Larsen
2023-03-08 14:16 ` Jan Beulich
2023-03-09 10:38 ` Matias Ezequiel Vara Larsen
2023-03-09 11:50 ` Jan Beulich
2023-03-10 10:58 ` Matias Ezequiel Vara Larsen
2023-03-10 11:34 ` Jan Beulich
2023-03-14 10:28 ` Matias Ezequiel Vara Larsen
2023-03-14 10:34 ` Jan Beulich
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230307144427.GA997565@horizon \
--to=matiasevara@gmail.com \
--cc=andrew.cooper3@citrix.com \
--cc=dfaggioli@suse.com \
--cc=george.dunlap@citrix.com \
--cc=jbeulich@suse.com \
--cc=julien@xen.org \
--cc=matias.vara@vates.fr \
--cc=roger.pau@citrix.com \
--cc=sstabellini@kernel.org \
--cc=wl@xen.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).